-
-
Notifications
You must be signed in to change notification settings - Fork 489
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
split out main menu code from studio #1813
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nesbox
requested changes
Jan 17, 2022
joshgoebel
force-pushed
the
start_simplify
branch
from
January 17, 2022 14:39
597f97f
to
2aea5fc
Compare
joshgoebel
force-pushed
the
start_simplify
branch
from
January 17, 2022 19:14
2aea5fc
to
83f4434
Compare
joshgoebel
commented
Jan 17, 2022
Can we merge your PR or are you going to make some other changes? |
It's good to go I think. I can make more PRs. Small PRs are good I think. |
nesbox
approved these changes
Jan 22, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the very beginning of my work to try and split the codebase up into smaller, more modular pieces. It follows our discussion about privacy and studio not having full access to
core
(even internally), etc... it seems to then follow that the menu menu shouldn't have full access to the hugeimpl
struct that is essentially our entire application (that currently resides in studio)... also studio has tended to be a bit of a "catch all" file, so pulling out functionality into separate files would be good thing on it's own I feel.This removes all menu menu related code from
studio
. It leaves all state management code in studio (impl.mode
)... the things that main menu still needs access to:init
)init
) - this would not be necessary if we movedrwConfig
tostudio
and just allowed access to that for consumers. We could also have headers for different privilege levels... so that you'd only includewrites_config.h
for things that neededr/w
access to the config, etc.init
)impl
for now)For
impl.gamepads
we could copy a pointer as well or add an API for that... Themem
access feels like something we could hide behind an API in the future (it's only used to getscript: menu
and for the menu callback), but I was trying to take things one step at a time.Splitting out
studio_impl.h
was necessary to avoid a circular dependency in the header files.The code in
main_menu.c
is 99% cut and paste from studio with just glue needed at the edges (initMainMenu
, etc.) and to make the references to our internal state rather than the global studioimpl
object.Commits: