Skip to content

presets: clear blocks#5098

Merged
aMannus merged 3 commits intoHarbourMasters:developfrom
serprex:preset-prefix
Feb 28, 2025
Merged

presets: clear blocks#5098
aMannus merged 3 commits intoHarbourMasters:developfrom
serprex:preset-prefix

Conversation

@serprex
Copy link
Contributor

@serprex serprex commented Feb 25, 2025

this avoids needing to list every new cvar in presets.h

fixes #4906

Build Artifacts

@serprex serprex force-pushed the preset-prefix branch 2 times, most recently from 0918c1b to dc6dcd4 Compare February 25, 2025 23:55
this avoids needing to list every new cvar in presets.h
@serprex serprex marked this pull request as draft February 26, 2025 00:23
@serprex serprex marked this pull request as ready for review February 26, 2025 00:46
Copy link
Contributor

@aMannus aMannus left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up! 2 small things, one of which is my own baguette (sowwy)

Comment on lines 66 to 68
CVAR_SETTING("DpadInText"),
CVAR_SETTING("OcarinaControl.Dpad"),
CVAR_SETTING("OcarinaControl.RStick"),
Copy link
Contributor

Choose a reason for hiding this comment

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

We should just get rid of the enhancement and randomizer cvar lists and only clear by blocks. The ones that were in this before shouldn't really have been reset anyway, and if any of these settings becomes a concern for racing down the line, they shouldn't live in settings anyway.

Means you can remove them where Malk placed a comment too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I also remove from presets things which don't fall into the blocks? ie vanillaPlusPresets sets CVAR_SETTING("DpadInText"), CVAR_SETTING("OcarinaControl.Dpad"), CVAR_SETTING("OcarinaControl.RStick")

Even randomizer, which only cleared randomizer blocks to begin with, touches settings/enhancements. I saw someone complain about not wanting to touch rando presets because of fear of their non-rando settings getting screwed up. But this might also be fine to punt on for now here since there's the follow up of #4851

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I don't think we should in the future. The problem with randomizer presets was it was also touching enhancements though, not just settings. But maybe it's a good rule to follow from now on. Enhancement presets can only touch stuff inside enhancements (and cheats but that'll move to the enhancement tab in the new menu anyway), randomizer preset can only touch randomizer settings etc.

I do foresee some settings we'd probably want to enable in these presets though, but we can hold a discussion if those need to be moved to enhancements later then.

@aMannus aMannus merged commit eb6c0d9 into HarbourMasters:develop Feb 28, 2025
20 of 25 checks passed
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.

Enhancements and randomizer settings missing from presets.h

3 participants