tmewett / BrogueCE

Brogue: Community Edition - a community-lead fork of the much-loved minimalist roguelike game
https://sites.google.com/site/broguegame/
GNU Affero General Public License v3.0
1.03k stars 109 forks source link

Simplify button-drawing logic #641

Closed Nathan-Fenner closed 10 months ago

Nathan-Fenner commented 10 months ago

This is a refactor of the way that buttons are drawn and some button-related events are handled. The main idea is to remove dbuf and rbuf from the buttonState struct, and instead just re-render the buttons whenever they need to be added to the screen.

This gets rid of a lot of low-level logic for highlighting/unhighlighting buttons, and makes the behavior of saving/restoring the screen clearer, because it becomes more local, i.e. now you'll see this logic explicitly:

// All in one block of code:

screenDisplayBuffer rbuf;
overlayDisplayBuffer(..., &rbuf); // save the original screen

// draw stuff & wait for input 
...

// Restore the screen to how it was before this block executed:
overlayDisplayBuffer(&rbuf, NULL);

effectively this is what was happening before, but figuring out the details required traveling through several layers of button-handling logic. In particular, it bothered me that the "initialize button state" function also drew to the button buffer. This doesn't need to happen anymore.


I accidentally found and fixed an input handling bug on the main menu: because of the animated flame background, there are a lot of pause(...) loops on the title screen.

The pause(...) family of functions return true if there are new input events to handle. However, in order to avoid cancelling auto-explore, the SDL backend treats mouse-move events as not having happened. This means that you don't get "hovered" styling for buttons in the main menu, because it can't see any mouse movement events during the flame animation!

Therefore, all pausing functions now take a PauseBehavior which allows the caller to decide whether they want to treat mouse movement events as interruptions or not. Almost every existing call uses the old behavior.

This affects platform implementations because their pause function must now read the provided PauseBehavior argument. However, supporting this should be largely trivial (i.e. I already fixed the SDL platform in about 2 lines of code).

tmewett commented 10 months ago

Button refactoring looks great.

I'm a v weak no on hover change, I never saw it as a bug, but if ZZ likes it he overrides

zenzombie commented 10 months ago

Button refactoring looks great.

I'm a v weak no on hover change, I never saw it as a bug, but if ZZ likes it he overrides

Ok cool. We can always change the hover later. I don't feel strongly about it.