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

Enhancement: Better Debug Warp MQ toggle #2876

Merged
merged 4 commits into from
May 30, 2023

Conversation

stratomaster64
Copy link
Contributor

@stratomaster64 stratomaster64 commented May 11, 2023

Resolves #2860.
I don't think using SceneContext->opt for determining the MQ toggle is harmful considering that a) it does nothing in vanilla and b) this option is only available with better debug warp, maintaining the stance of keeping things authentic when possible.

Build Artifacts

@Archez
Copy link
Contributor

Archez commented May 11, 2023

Part of me wonders if this MQ selection behavior should be restricted to fileNum 255 (when using debug warp from title screen/file select, instead of within a valid save).

As it stands gBetterDebugWarpScreenMQFlag behaves one way when in a vanilla save, and behaves the opposite for a MQ save, which feels odd to me..
The other thing is this doesn't account for randomizer saves with mixed MQ and vanilla dungeons.

If we wanted the MQ toggle to work within saves too, I think the MQ toggle should automatically update and match the default status of the save (defaults to MQ in MQ saves, and defaults to the correct dungeon type in mixed rando saves). Then when navigating up/down between the dungeons itll update to match the correct default status.

This way someone testing warping to dungeons in a rando save will always be directed to the expected dungeon without having to read the spoiler log.

I'm curious to hear others thoughts on this.

@stratomaster64
Copy link
Contributor Author

As it stands gBetterDebugWarpScreenMQFlag behaves one way when in a vanilla save, and behaves the opposite for a MQ save, which feels odd to me.. The other thing is this doesn't account for randomizer saves with mixed MQ and vanilla dungeons.

I'm currently looking at that now. It seems that on vanilla saves, it works properly but on MQ saves, it just forces MQ no matter what,

If we wanted the MQ toggle to work within saves too, I think the MQ toggle should automatically update and match the default status of the save (defaults to MQ in MQ saves, and defaults to the correct dungeon type in mixed rando saves). Then when navigating up/down between the dungeons itll update to match the correct default status.

This shouldn't be too hard to implement, I think,

@Archez
Copy link
Contributor

Archez commented May 11, 2023

As it stands gBetterDebugWarpScreenMQFlag behaves one way when in a vanilla save, and behaves the opposite for a MQ save, which feels odd to me.. The other thing is this doesn't account for randomizer saves with mixed MQ and vanilla dungeons.

I'm currently looking at that now. It seems that on vanilla saves, it works properly but on MQ saves, it just forces MQ no matter what,

It might be easier to have two CVars, one that says which type to load, and one that says to override the scene loading process.

@stratomaster64
Copy link
Contributor Author

Upon further investigation, there doesn't seem to be a good way to grab gRandomizer's MQ dungeon list and bring it over to z_select.

@Archez
Copy link
Contributor

Archez commented May 11, 2023

Upon further investigation, there doesn't seem to be a good way to grab gRandomizer's MQ dungeon list and bring it over to z_select.

You should be able to use ResourceMgr_IsSceneMasterQuest. But you will need to remove the gPlayState check from the IsSceneMasterQuest method. Historically, we pulled the current scene value of play state hence the need of a null check, but it has since then been changed to accept the scene num as a param to the method. This renders the play state null check unneeded now.

Item tracker and Check tracker have an example of using extern C to expose ResourceMgr_IsSceneMasterQuest into a .cpp file.
And you will probably need to set the scene IDs somewhere in the data structs.

@briaguya-ai
Copy link
Contributor

@stratomaster64 is this ready for another review?

@stratomaster64
Copy link
Contributor Author

@stratomaster64 is this ready for another review?

Go for it!

@Archez
Copy link
Contributor

Archez commented May 30, 2023

You should be able to use ResourceMgr_IsSceneMasterQuest. But you will need to remove the gPlayState check from the IsSceneMasterQuest method.

Item tracker and Check tracker have an example of using extern C to expose ResourceMgr_IsSceneMasterQuest into a .cpp file.

And you will probably need to set the scene IDs somewhere in the data structs.

I'm still interested in this being done to make the warping more intelligent based on my original comment above. Is this something you'd be willing to do in this PR @stratomaster64 ? Otherwise I could handle it in a follow up PR.

@stratomaster64
Copy link
Contributor Author

You should be able to use ResourceMgr_IsSceneMasterQuest. But you will need to remove the gPlayState check from the IsSceneMasterQuest method.
Item tracker and Check tracker have an example of using extern C to expose ResourceMgr_IsSceneMasterQuest into a .cpp file.
And you will probably need to set the scene IDs somewhere in the data structs.

I'm still interested in this being done to make the warping more intelligent based on my original comment above. Is this something you'd be willing to do in this PR @stratomaster64 ? Otherwise I could handle it in a follow up PR.

You can make a follow-up PR to this, I'm working on a few other things so I probably wouldn't be able to get to it on time.

Copy link
Contributor

@leggettc18 leggettc18 left a comment

Choose a reason for hiding this comment

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

I think this could do with some future cleanup about hiding the MQ option when it's not applicable (only one OTR) but I don't want to consider it a blocker since picking the "incompatible" option does correctly just do nothing right now.

@leggettc18 leggettc18 merged commit 507387f into HarbourMasters:develop May 30, 2023
7 checks passed
@stratomaster64 stratomaster64 deleted the mq-debug-menu branch June 6, 2023 01:32
@garrettjoecox garrettjoecox added this to the Sulu (7.1.x) milestone Jun 13, 2023
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.

Enhancement - Select Master Quest Dungeons in Debug Warp Screen
5 participants