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

Explore not override the menu ui completely #241

Open
rikkolovescats opened this issue Feb 27, 2024 · 0 comments
Open

Explore not override the menu ui completely #241

rikkolovescats opened this issue Feb 27, 2024 · 0 comments
Labels
enhancement New feature or request plugin-manager Everything plugin manager

Comments

@rikkolovescats
Copy link
Collaborator

rikkolovescats commented Feb 27, 2024

Right now, we show the plugin manager button (the one with the store icon) in the game's settings window. The way this is currently implemented is in that it completely overwrites the vanilla menu implementation to rebuild the menu with the plugin manager entry in it. This isn't the best way to do it, since if there are any other plugins that want to show their own entry in main menu (similar to plugin manager), then this will end up causing a race condition where which ever plugin gets loaded the last will supersede any parent implementations of the main menu. And in the case if plugin manager is the last plugin to load up, then we'll end up overwriting the previous menu overrides from any earlier loaded plugins.

A better idea should be to override only the very specific methods of the main menu in a way that reuses as much of the vanilla code as possible. Something along these lines say when overriding the _save_state method. The current implementation in plugin manager completely overwrites the parent method:

def _save_state(self) -> None:
try:
sel = self._root_widget.get_selected_child()
if sel == self._controllers_button:
sel_name = 'Controllers'
elif sel == self._graphics_button:
sel_name = 'Graphics'
elif sel == self._audio_button:
sel_name = 'Audio'
elif sel == self._advanced_button:
sel_name = 'Advanced'
elif sel == self._modmgr_button:
sel_name = 'Mod Manager'
elif sel == self._back_button:
sel_name = 'Back'
else:
raise ValueError(f'unrecognized selection \'{sel}\'')
assert bui.app.classic is not None
bui.app.ui_v1.window_states[type(self)] = {
'sel_name': sel_name
}
except Exception:
logging.exception('Error saving state for %s.', self)

Instead, a better approach could be to do something like this:

class NewAllSettingsWindow(AllSettingsWindow):
    ...

    def _save_state(self) -> None:
        sel = self._root_widget.get_selected_child()
        if sel == self._modmgr_button:
            sel_name = 'Mod Manager'
            assert bui.app.classic is not None
            bui.app.ui_v1.window_states[type(self)] = {
                'sel_name': sel_name
            }
        else:
            return super()._save_state()

Still need more exploration here to see if this approach can fit well with when overriding all the other remaining methods in AllSettingsWindow. but the idea here is that such changes will help plugin manager to co-exist with other main menu overriding plugins.

@rikkolovescats rikkolovescats added enhancement New feature or request plugin-manager Everything plugin manager labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request plugin-manager Everything plugin manager
Projects
None yet
Development

No branches or pull requests

1 participant