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

Add gRegEditEnabled #3173

Merged
merged 3 commits into from Oct 30, 2023
Merged

Conversation

Pepe20129
Copy link
Contributor

@Pepe20129 Pepe20129 commented Sep 5, 2023

Adds a cvar that enables the Registry Editor.

It should always be enabled but it checks for gIsCtrlr2Valid which is not updated properly, it probably has been broken since the initial porting.

Build Artifacts

Copy link
Contributor

@Archez Archez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

To add context, I dug into this a couple weeks ago and found that basically there is two things missing for this to have worked as intended.

  1. We are not setting the controller type in padmgr.c as the "normal" type
  2. When initializing the padmgr and the LUS controller side, LUS does some things asynchronously and overwrites a value to 0, causing the gIsCtrlr2Valid global to be not set. In the snippet below, outMask is immediately set to 0 by LUS, and then later is updated async.
    *outMask = 0xFF;
    ret = osContInit(mq, outMask, status);
    if (ret != 0) {
    return ret;
    }
    if (*outMask == 0xFF) {

With the addition of other ports leveraging LUS and the potential for other supported peripherals, we may want to consider updating LUS to report these things for us, like how we handled rumble support.

All that to say, I think having a dedicated CVar is totally fine here 👍 as ultimately, even if LUS was updated, the padmgr setup code is executed only once on launch, so requiring someone to re-launch with two controllers connected just for this is overkill.

@@ -1349,6 +1349,10 @@ void DrawDeveloperToolsMenu() {
{
UIWidgets::EnhancementCheckbox("OoT Debug Mode", "gDebugEnabled");
UIWidgets::Tooltip("Enables Debug Mode, allowing you to select maps with L + R + Z, noclip with L + D-pad Right, and open the debug menu with L on the pause screen");
if (CVarGetInteger("gDebugEnabled", 0)) {
UIWidgets::EnhancementCheckbox("OoT Registry Editor", "gRegEditEnabled");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we prefix with debug here gDebugRegEditEnabled. We are trying to categorize CVars under umbrellas, and I think it would be good to break out other debug features into similar sub-toggles.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be preferred to use gDebug.RegEditEnabled so that all the cvars we add under debug are indented together in the json?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think thats fine, but I'll let @Malkierian confirm since they are working on nesting our existing CVars.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can revisit when we get here, the cvar renames haven't been finalized yet

soh/soh/SohMenuBar.cpp Outdated Show resolved Hide resolved
@garrettjoecox garrettjoecox merged commit e4cfc88 into HarbourMasters:develop Oct 30, 2023
8 checks passed
@Pepe20129 Pepe20129 deleted the reg_editor branch December 22, 2023 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants