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.

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

Migrate remaining menus to ui_adaptor and clean up redundant drawing code #41112

Merged
merged 13 commits into from
Jun 6, 2020

Conversation

Qrox
Copy link
Contributor

@Qrox Qrox commented Jun 6, 2020

Summary

SUMMARY: Interface "All UIs now use the ui_adaptor system for refreshing and resizing"

Purpose of change

I did an extensive search of characteristic UI functions used in the game's UIs (for example wrefresh, draw_ter, refresh_display etc) and migrated all menus I could find that has yet been migrated. In the mean time I removed a bunch of redundant drawing code, which was there to refresh underlying menus but is no longer needed because refreshing is now handled entirely by ui_adaptor. I also fixed a few UI issues that I encountered during the migration process.

There might still be menus that I missed, but their implemention have to be extremely peculiar for me to not have found them.

Closes #8514.

Describe the solution

Encapsulate UI code with ui_adptor callbacks and game::draw_callback_t, and remove UI code that is no longer needed.

Testing

I tested every menu affected by this PR, and they were working as expected.

@Qrox Qrox requested a review from KorGgenT as a code owner June 6, 2020 10:25
@mlangsdorf mlangsdorf added the Info / User Interface Game - player communication, menus, etc. label Jun 6, 2020
@Leland
Copy link
Contributor

Leland commented Jun 6, 2020

Heck yeah, 32 PRs later. Nice job.

Should finally put the nail in the coffin for #8514 if you want to add that to the description.

@jbytheway
Copy link
Contributor

jbytheway commented Jun 6, 2020

Nice work!

Now that you've completed this migration, do you play to somehow enforce this for future UI code, or do you think it's sufficient that people will just copy the code that's there already?

@Qrox
Copy link
Contributor Author

Qrox commented Jun 6, 2020

It'll likely be hard to tell whether a piece of code should be wrapped in ui_adaptor or not, so I don't think there's a very practical way to enforce it automatically. The best approach I can think of so far is to tag functions with attributes (redraw only, event loop, etc) and mandate that event loop functions do not contain non-encapsulated UI code and redraw functions do not contain input code, but I'm not sure how practical is that.

I do plan to write some documentation for ui_adaptor though, which should suffice when people want to add more UIs in the future.

@ZhilkinSerg ZhilkinSerg merged commit 0423c66 into CleverRaven:master Jun 6, 2020
@anothersimulacrum anothersimulacrum added [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style labels Jun 6, 2020
@Qrox Qrox deleted the migrate-ui-25 branch June 7, 2020 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style Info / User Interface Game - player communication, menus, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Several keybinding menus missing redraws when closed
6 participants