Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ship window improvements (rebased) #6422

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Ship window improvements (rebased) #6422

wants to merge 7 commits into from
+35 −23

Conversation

bunnybot
Copy link

tothxaMirrored from Codeberg
Created on Fri Mar 29 02:17:34 CET 2024 by Tóth András (tothxa)

Type of change
Bugfix & New feature

Issue(s) closed
Replaces #6331 (rebased on master from protected/navalwarfare)

Fixes #6330

New behavior

  • only display SoldierPanel for warships that are ready to fight, show onboard wares display (possibly with soldiers as generic cargo) while refitting
  • allow stopping expeditions where they are (like warships) when they are scouting, only show construct port button when active

Possible regressions
Ship window, wares and workers handling while refitting and soldier capacity is set to 0

Additional context
Expedition stopping is in separate commit(https://codeberg.org/tothxa/widelands/commit/afc16dd84f78a31c3e2aad88c0f3897c58c6c8b2), so it can be reverted easily if voted down. Otherwise icons should be more different probably. Either use scaled down portspace icon for construct port, or a smaller anchor in the stopping icon?

@bunnybot bunnybot added this to the v1.3 milestone Mar 29, 2024
@bunnybot bunnybot self-assigned this Mar 29, 2024
@bunnybot
Copy link
Author

Assigned to tothxa

@bunnybot bunnybot added bug Something isn't working enhancement New feature or request military Soldiers, military sites, training sites & battles seafaring Ships, ports, naval invasions, … accessibility Color contrast, font size, user experience, etc. ui User interface ci:success CI checks succeeded labels Mar 29, 2024
@bunnybot bunnybot added ci:success CI checks succeeded and removed ci:success CI checks succeeded labels Apr 21, 2024
@bunnybot bunnybot added ci:success CI checks succeeded and removed ci:success CI checks succeeded labels May 1, 2024
@bunnybot bunnybot added ci:success CI checks succeeded and removed ci:success CI checks succeeded labels May 14, 2024
@bunnybot bunnybot added ci:success CI checks succeeded and removed ci:success CI checks succeeded labels May 19, 2024
@bunnybot bunnybot added ci:success CI checks succeeded and removed ci:success CI checks succeeded labels May 30, 2024
@bunnybot
Copy link
Author

tothxaMirrored from Codeberg
On Thu May 30 23:43:30 CEST 2024, Tóth András (tothxa) wrote:

In my opinion the two icons look different enough.

Here's the expedition window with port_0.5.png copied to ship_construct_port_space.png:

construct_port_scrrenshot.png

I quite like it, though haven't tested in a longer game. But if needed, we now have the blender model, so we can scale it down slightly. Or should I add the downward arrow too?

One small-ish thing that should be fixed: When clicking Stay for an expedition while in sight of a port space, the Build Port button should become available again. Currently the only way to build a port after sailing even a single field away from it is to go very far away until it is out of sight and then return.

OK, I'll try to do it.

@tothxa
fix Ship::exp_scouting_direction() for WalkingDir::IDLE
1915a2c 
re-enables construct port button when ship is stopped while still seeing a portspace
@bunnybot bunnybot added ci:success CI checks succeeded and removed ci:success CI checks succeeded labels May 30, 2024
@bunnybot
Copy link
Author

tothxaMirrored from Codeberg
On Fri May 31 02:03:36 CEST 2024, Tóth András (tothxa) wrote:

Oops, I didn't mean to commit the icon change without approval. Sorry!

@bunnybot
Copy link
Author

NordfrieseMirrored from Codeberg
On Fri May 31 13:22:24 CEST 2024, Benedikt Straub (Nordfriese) wrote:

Works 👍

One more nit, after giving the command to construct a port, the Stay button remains enabled for a second or so before the ship reverts to a regular Transport ship. Pressing this button at this time results in a log message

WARNING:  1:ship on  25x 32 received scout command but not in kExpeditionWaiting or kExpeditionPortspaceFound or kExpeditionScouting status (expedition: Y), ignoring...

I suppose this could be fixed by disabling the stay button when the ship is colonizing.

I'm not really happy with the new Construct Port icon, IMHO it looks a bit fuzzy and lacking contrast at this scale. Maybe overlay the old anchor icon above it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bunnybot bunnybot

Labels
accessibility Color contrast, font size, user experience, etc. bug Something isn't working ci:success CI checks succeeded enhancement New feature or request military Soldiers, military sites, training sites & battles seafaring Ships, ports, naval invasions, … ui User interface
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.

Warship soldier capacity can't be set to 0 if ship still has wares
2 participants
@bunnybot @tothxa