Skip to content

Conversation

@Zakru
Copy link
Contributor

@Zakru Zakru commented Jun 11, 2023

Fixes #376

@Indy2222
Copy link
Collaborator

I figured I'd ask feedback for the layout here. For example would a constant square container be preferable as opposed to the previous 20%x30% screen size? The current fix does not currently work for a wider aspect ratio (stretches to fill the container, like main branch), and my solution depends on the final layout.

I think that a good solution should have these properties:

  • The black container (which contains the map) is wider by min(x * screen_width, y * screen_height) than the minimap.
  • The minimap is centered in the black container.
  • The minimap is a rectangle with the same aspect ratio as the game map.
  • The minimap is the larges rectangle with the above properties whose height is not over 20% of the screen height and width over 30% screen width.

@Zakru
Copy link
Contributor Author

Zakru commented Jun 11, 2023

Does this somewhat illustrate what you are thinking? I corrected the order of the percentages assuming you meant how it currently is (confusing since 30% ends up being less than 20% on a 16:9). Also, if the width is responsive, I take it we should also include the action bar in this layout?
tall minimap concept
wide minimap concept

@Indy2222
Copy link
Collaborator

Does this somewhat illustrate what you are thinking? I corrected the order of the percentages assuming you meant how it currently is (confusing since 30% ends up being less than 20% on a 16:9). Also, if the width is responsive, I take it we should also include the action bar in this layout?

Yes, your drawing illustrates well what I meant. You are right about the action bar, it should horizontally end where the minimap begins.

@Indy2222
Copy link
Collaborator

Note that this also solves part of #377 and is distantly related to #345.

@Zakru
Copy link
Contributor Author

Zakru commented Jun 16, 2023

Got a bit stuck on this one, it seems to me like the aspect ratio constraint works only in only one direction while the other has a size constraint, but other than that it might not be able to "expand to fit" with just the vanilla bevy_ui. Tried asking for help but got nothing of use. Any suggestions from here? Custom controller etc?

Also feedback on the new code layout is welcome, since the bottom HUD is now a flexbox so a hierarchy is needed. Right now, I have the container defined in hud/mod.rs while the parts are created from code in their respective files.

@Indy2222
Copy link
Collaborator

Got a bit stuck on this one, it seems to me like the aspect ratio constraint works only in only one direction while the other has a size constraint, but other than that it might not be able to "expand to fit" with just the vanilla bevy_ui. Tried asking for help but got nothing of use. Any suggestions from here? Custom controller etc?

I would try to explore the padding trick (https://css-tricks.com/aspect-ratio-boxes/).

If that doesn't lead anywhere, we might need to implement a system which on change of the window resolution recalculates the sizes of the nodes and changes them. This should be pretty simple system and a small modification.

Also feedback on the new code layout is welcome, since the bottom HUD is now a flexbox so a hierarchy is needed. Right now, I have the container defined in hud/mod.rs while the parts are created from code in their respective files.

I would try to minimize changes in this PR and postpone any unnecessary code structuring changes to a separate PR.

I'll do a proper review once we arrive at a solution which works. I quickly tried to find a simple solution in which we do not have to (re)calculate the size in a system, but without success.

@Zakru
Copy link
Contributor Author

Zakru commented Jun 17, 2023

I would try to minimize changes in this PR and postpone any unnecessary code structuring changes to a separate PR.

I also actually wish to not do this here. The logic was that technically when the minimap size changes, there's a gap/overlap between it and the action bar, but I guess that might be a non-issue at this point.

I would try to explore the padding trick (https://css-tricks.com/aspect-ratio-boxes/).

Couldn't get this to work, might just be me, although I think it's the fact that we want to constrain the size in both directions while maintaining the aspect ratio, and have it expand to the max size possible. Everything I've seen just shows a size constraint on one axis, while the aspect ratio controls the other.

@Indy2222
Copy link
Collaborator

I would try to minimize changes in this PR and postpone any unnecessary code structuring changes to a separate PR.

I also actually wish to not do this here. The logic was that technically when the minimap size changes, there's a gap/overlap between it and the action bar, but I guess that might be a non-issue at this point.

I would try to explore the padding trick (https://css-tricks.com/aspect-ratio-boxes/).

Couldn't get this to work, might just be me, although I think it's the fact that we want to constrain the size in both directions while maintaining the aspect ratio, and have it expand to the max size possible. Everything I've seen just shows a size constraint on one axis, while the aspect ratio controls the other.

We will limit the aspect ration for maps anyways (#375) so setting just height (or width) and leaving the other dimension to grow/shrink might be sufficient. Would that be easier to achieve?

@Zakru
Copy link
Contributor Author

Zakru commented Jun 17, 2023

We will limit the aspect ration for maps anyways (#375) so setting just height (or width) and leaving the other dimension to grow/shrink might be sufficient. Would that be easier to achieve?

I think that would be significantly easier. For example, fix the width to 20% of the screen, and have the maps only be wide rectangles (or do the opposite and fix the height, whichever). With this approach, even if we allow the opposite aspect ratio, we could just rotate the map a quarter turn and get the shape we want. Would also make the visuals more consistent imo.

@Indy2222
Copy link
Collaborator

We will limit the aspect ration for maps anyways (#375) so setting just height (or width) and leaving the other dimension to grow/shrink might be sufficient. Would that be easier to achieve?

I think that would be significantly easier. For example, fix the width to 20% of the screen, and have the maps only be wide rectangles (or do the opposite and fix the height, whichever). With this approach, even if we allow the opposite aspect ratio, we could just rotate the map a quarter turn and get the shape we want. Would also make the visuals more consistent imo.

Ok, so lets rework the implementation in this PR so that the width is fixed at 20%. We can limit aspect ratio of the maps from 2:1 to 1:1.

@Zakru Zakru marked this pull request as ready for review June 17, 2023 21:43
For now we only constrain width.

Fixes DigitalExtinction#376
@Indy2222 Indy2222 added this pull request to the merge queue Jun 17, 2023
Merged via the queue into DigitalExtinction:main with commit f4eb6a4 Jun 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Draw minimap with appropriate aspect ratio

2 participants