game window now resizes based on sidebar size. no more manual offsets.

[esotericist]

Summary

SUMMARY: Interface "game window now resizes based on sidebar size. no more manual offsets."

Purpose of change

There's been a steady-state of various 'slightly off, but only sometimes' issue reports since the UI rewrite, due to the change in having the main game window take up part of the screen (where the sidebar wasn't), to it taking up all of the screen (with the sidebar rendered on top).

In addition, while this doesn't show up in the Tiles build the same way due to how buffering works, in curses there was a "z-fighting" type effect where the play area and sidebar would be drawing over each other, creating a flickering problem.

Fixes #29619
Fixes #28863

I have suspicions about #29627 that this PR will probably address, although without a currently-available failstate example I can't prove it.
With #29593 , this doesn't... fix it, exactly? I think there's an error in the related logic that causes it to not actually flash the appropriate tile. But at least it won't be flashing somewhere off to the side anymore. See 'Additional Context' below.
If #29184 isn't fixed already, it should be by this point (but obviously confirmation will be needed).

Describe the solution

Adjusted game::init_ui() to adjust the main game window size based off of edge offsets, which are currently controlled by the panels code (and currently supports things taking up both the right and left edges of the screen). Also added a call to game::init_ui() from the panels configuration screen (for when the layout changes)

Removed every existing reference to sidebar_offset because it's no longer the same concept as it used to be.

Further note on the sidebar_offsets struct in use here: The current game logic should work absolutely correctly with panels of arbitrary widths rendered on both the left and right sides of the screen, if the panels code (or some other UI mechanism) correctly sets the appropriate edge offsets. There shouldn't be any need to chase this down in game logic in the future, and it would be trivial to extend this to support top and bottom (such as for things like the "hotkey bar" in android, or if someone wanted UI panels that pinned to top/bottom of the screen) if needed.

Describe alternatives you've considered

We could have continued down the path of chasing down all of the offsets by hand, but that was both unsustainable and was overall increasing maintenance burden.

Additional context

I have tested this in the tiles build both with and without tiles enabled, as well as in the curses build. As far as I can tell, everything we've been struggling with for offsets recently should be properly addressed in a final, generalized fashion.

I tested: looking around, firing weapons at monsters, throwing weapons at monsters, making zones, driving vehicles. I'll be genuinely surprised at this point if there's anything still problematic.

A gif of me shooting a zombie hulk in true curses mode:

It's a close up, but I can promise nothing else is showing up on-screen outside of that window.

[KorGgenT]

code looks good to me.

[codemime]

This looks much like feature envy plus a bidirectional dependency: game operates on panel_manager's instance, whilst the later shoves its data back into the game class: game::draw_panels ---> panel_manager::draw_adm ---> panel_manager::update_offsets ---> game::sidebar_offsets

[esotericist]

I don't know what 'feature envy' means.

as for the bidirectional dependency, i was keeping data approximately where it was before, I genuinely didn't know what else to do with it that'd be more correct, especially with how panels is structured

[kevingranade]

https://softwareengineering.stackexchange.com/questions/212128/what-is-a-feature-envy-code-and-why-is-it-considered-a-code-smell

Basically, if the game owns these offsets, then panel manager should just inform game about the update and have it handle fetching the options etc.

Possibly just by having game::init_ui() accept a with argument, or by having game::uinit_ui() fetch it directly with panel_manager::get_manager().get_current_layout().begin()->get_width();

[esotericist]

Moved the data into panel_manager as private members, added public getters. addresses the bidirectional dependency, improves clarity as to what the data is.

[esotericist]

I'm not in a position to resolve the bidirectional dependency aspect right now, so I think this is as done as I can get it for the time being.

[esotericist]

This is definitely not ready because I have a NPE case I'm trying to chase down.

[esotericist]

So, I don't know if this is still "correct" (and in particular I'm aware that the panels do not yet provide both left and right side widths simultaneously), but init_ui() is now pretty uncaring about sidebar position and width which I think is an improvement.

I also renamed the members in panels.cpp when I moved that data out of game.cpp, to be a bit clearer (there was a bit of a nomenclature confusion in discord discussion involving "offset" that was linked to how this stuff used to work). I'm hoping width_left and width_right are fairly clear in purpose within the context of the panels.

Of particular note is the incredibly massive mistake I made directly altering VIEW_OFFSET_X with the left sidebar width, because that gets translated to a different coordinate system directly thereafter. I now alter the call to catacurses::newwin() and this prevents an overflow later on when calling invalidate_framebuffer() in sdltiles.cpp. It also reflects how init_ui() handled this before the UI rework.

I'm honestly astonished I ever got it to work with the sidebar in the left in my previous tests. I am certain I tested it, I am not certain how it didn't explode violently the way it has for the last few hours (before I finally figured out my mistake on VIEW_OFFSET_X). Such stacktrace. Very confuse.

Hopefully there aren't any strong opinions on the state of this PR at this point. This is easily the most exhausting thing I've attempted for this game so far, but I hope it's still worth the effort.

[ifreund]

Over all looks good, just a couple things

[ifreund]

looks like width left is always 0? If it works like this, why are we even storing this? Or is this a typo?

[esotericist]

That's to allow for panels to decide to use some space on the right and space on the left if need be. It isn't currently used but my understanding is panels will soon have the ability to not be restricted to one side of the screen.

That's why there's two values, and why get_width_right() and get_width_left() are mirrors at present. It's to ensure that game.cpp doesn't have to know where the sidebar is, only which parts of the screen are available for its use after the reduction of space for the sidebar(s).

[ifreund]

Cool, that makes sense to me

[esotericist]

Aight. Whether this is usable as it is or not, I think I have to be done with this one. It's as ready as I'm able to make it.

[ifreund]

Fixes #29741