Rando settings streamline #3391

leggettc18 · 2023-11-13T15:44:06Z

Depends on rando-gap-bridging PR.

Closes some of the gap between 3drando settings and CVars/ImGui.
Options now can store a CVar Name and have functions to set the CVar according to their value or vice versa.
Options can use that info plus new enum values and flags to automatically render an ImGui Widget.
OptionGroups can use this new info plus a new enum value for them to render groups of Options as tables/columns/sections, etc.
All of this enabled removal of the cvarSettings array and a significant amount of code from RandomizerSettingsWindow::DrawElement. Eventually even more code can be dropped but it will take a significant amount more work to port/relocate the logic for Excluded Locations and Tricks.

Build Artifacts

jbodner09 · 2023-11-13T20:58:17Z

soh/soh/Enhancements/randomizer/3drando/hints.cpp

+  for (int i = 0; i < LANGUAGE_MAX; i++) {
+    std::string textStr = unformattedHintText.GetForLanguage(i);
+    // RANDOTODO: don't just make manual exceptions
+    bool needsAutomaicNewlines = true;


This actually isn't my typo. Copied it in from spoiler_log.cpp, since I actually needed the formatting it applied earlier than when it entered the spoiler file after changes.

Also I think this only appeared as a change in this PR because it was opened before the rando-gap-bridging PR got merged, so it showed all of those changes as well. I've since rebased so I don't think it actually shows up as a change anymore.

Also it's out of scope here but the only reason we actually ever needed that variable is due to a bug. So I'll have a separate cleanup PR in the near future that will include that bugfix which will result in that line no longer existing.

jbodner09 · 2023-11-13T21:05:41Z

soh/soh/Enhancements/randomizer/3drando/playthrough.cpp

@@ -58,10 +59,10 @@ int Playthrough_Init(uint32_t seed, std::unordered_map<RandomizerSettingKey, uin
    GenerateHash();
    WriteIngameSpoilerLog();

-    if (Settings::GenerateSpoilerLog) {
+    if (/*Settings::GenerateSpoilerLog TODO: do we ever not want to write a spoiler log?*/ true) {


Race seeds?

tl;dr I agree, but actually implementing that option involves some work that is out of scope for this PR.

Yeah I remember your comment on the other PR. I just haven't gone through and implemented that option in the new system yet, felt out of scope here and would be done in a cleanup PR of some kind soon. There's also some other changes involved in that, for example with race seeds I think we would still want to generate a json file, it would just only have the settings and the seed string. That way we can drag and drop to gen the same seed for races without sharing the entire spoiler log.

jbodner09 · 2023-11-13T21:10:28Z

soh/soh/Enhancements/randomizer/context.cpp

+    for (auto& item : StaticData::GetItemTable()) {
+        // Easiest way to filter out all the empty values from the array, since we still technically want the 0/RG_NONE
+        // entry
+        if (item.GetName().english.empty())


I believe our new coding standards require brackets for all if statements even if they're a single line.

Good Catch! This code probably came from somewhere else via copy paste and was already like that so I didn't catch it.

Although now that I'm looking closer at it, this is no longer a change in this PR post rebase, as it was in the previous rando-gap-bridging PR. So this will happen as part of a separate "tidy-up" PR, alongside some of the stuff from your other comments.

Options now link with CVar names directly. So instead of passing the cvarSettings map, the Option class can check a corresponding CVar if a cvarName was provided during construction. Of note, it does not automatically sync the Option selected index with the CVar value, as we would not want this to happen in all cases, for example when dragging a spoiler file, we don't want to overwrite all the CVars with the Options from the spoiler file. Currently all Options are set to the value of the CVar they are linked to right before generating a new seed, unless a spoiler file has been dropped in which case those settings are used instead.

Currently only the slider variant. Will allow for auto rendering of options in ImGui, with tooltips and automatic display of the values in each Option's options array while keeping the CVars at the selected index, preventing Off By One Errors.

Currently only in use for a couple of items, future commit will implement for all options.

With the exception of the Locations and Tricks tabs, those are special and will need a lot more work.

These descriptions will automatically display as tooltips in ImGui, if the widget container type accounts for it.

Trying out CLion Nova, and it highlighted some things I decided to fix, some from CLion itself and some from CLang-Tidy. Oddly, it didn't like a conversion from size_t to int whether I left it implicit or added a static_cast, so I guess that warning is staying.

Specifically we went from storing the actual value in the CVar to storing an index, meaning sliders that started with 1 now have the index offset by 1. This is currently only Big Poe Count, Triforce Hunt total/required, and starting hearts. Everything else either already started at 0, or in the case of LACS/Bridge counts, we were starting the sliders at 1 but they would have always worked at 0 according to the 3drando logic.

leggettc18 · 2023-11-17T18:31:33Z

I opened this based on a prior PR that had not been merged yet, it was merged a little while ago so I rebased it just now.

…pwright into rando-settings-streamline

leggettc18 · 2023-11-21T19:28:37Z

Taking this one out of draft. I might see if I can streamline the excluded locations and tricks UI a bit before this merges but it might be better saved for a separate PR, this one is already large.

leggettc18 · 2023-11-28T17:02:41Z

soh/soh/Enhancements/randomizer/randomizer.cpp

+    if (changed) {
+        ctx->GetSettings()->UpdateOptionProperties();
+    }


I'm now realizing that RandomizerSettingsWindow has an unused UpdateElement function which could probably be used here, instead of calling this function (responsible for updating Disabled/Hidden and certain gui flags). That might be an easy enough change to get into this PR, will check.

I actually just pushed a commit to address this. RandomizerSettingsWindow now has a property for keeping track of whether or not it needs an update, which UpdateElement then checks and runs the Settings::UpdateOptionProperties function.

leggettc18 · 2023-11-28T17:07:52Z

soh/soh/Enhancements/randomizer/randomizerTypes.h

+    RSG_AREA_ACCESS_IMGUI,
+    RSG_WORLD_IMGUI,
+    RSG_SHUFFLE_ENTRANCES_IMGUI,
+    RSG_WORLD_IMGUI_TABLE,
+    RSG_SHUFFLE_ITEMS_IMGUI,
+    RSG_SHUFFLE_NPCS_IMGUI,
+    RSG_SHUFFLE_DUNGEON_ITEMS_IMGUI,
+    RSG_ITEMS_IMGUI_TABLE,
+    RSG_TIMESAVERS_IMGUI,
+    RSG_ITEM_POOL_HINTS_IMGUI,
+    RSG_EXTRA_HINTS_IMGUI,
+    RSG_ITEM_POOL_HINTS_IMGUI_COLUMN,
+    RSG_ADDITIONAL_FEATURES_IMGUI,
+    RSG_GAMEPLAY_IMGUI_TABLE,
+    RSG_STARTING_EQUIPMENT_IMGUI,
+    RSG_STARTING_ITEMS_IMGUI,
+    RSG_STARTING_NORMAL_SONGS_IMGUI,
+    RSG_STARTING_WARP_SONGS_IMGUI,
+    RSG_STARTING_SONGS_IMGUI,
+    RSG_STARTING_INVENTORY_IMGUI_TABLE,


I made separate groups specifically for organizing options to render them in ImGui. This leads to some duplication between these and the previously existing ones in terms of what options they hold, but the idea is that one set is for organizing how those options appear in the spoiler file, while this set is for organizing how they appear in ImGui. We could probably just use this new set for both, but it would require a lot of changes to spoiler parsing so I would prefer to do that in a followup PR.

Also worth remembering, OptionGroups store pointers to Options or other OptionGroups, so they don't take up a lot of space.

leggettc18 · 2023-11-28T17:10:18Z

soh/soh/Enhancements/randomizer/settings.cpp

+        &mOptions[RSK_KEYRINGS_FOREST_TEMPLE],
+        &mOptions[RSK_KEYRINGS_FIRE_TEMPLE],
+        &mOptions[RSK_KEYRINGS_WATER_TEMPLE],
+        &mOptions[RSK_KEYRINGS_SPIRIT_TEMPLE],
+        &mOptions[RSK_KEYRINGS_SHADOW_TEMPLE],
+        &mOptions[RSK_KEYRINGS_BOTTOM_OF_THE_WELL],
+        &mOptions[RSK_KEYRINGS_GTG],
+        &mOptions[RSK_KEYRINGS_GANONS_CASTLE],
+    }, false, WidgetContainerType::COLUMN);
+    mOptionGroups[RSG_ITEMS_IMGUI_TABLE] = OptionGroup::SubGroup("Items", {
+        &mOptionGroups[RSG_SHUFFLE_ITEMS_IMGUI],
+        &mOptionGroups[RSG_SHUFFLE_NPCS_IMGUI],
+        &mOptionGroups[RSG_SHUFFLE_DUNGEON_ITEMS_IMGUI],
+    }, false, WidgetContainerType::TABLE);
+    mOptionGroups[RSG_TIMESAVERS_IMGUI] = OptionGroup::SubGroup("Timesavers", {
+        &mOptions[RSK_CUCCO_COUNT],
+        &mOptions[RSK_BIG_POE_COUNT],
+        &mOptions[RSK_SKIP_CHILD_STEALTH],
+        &mOptions[RSK_SKIP_CHILD_ZELDA],
+        &mOptions[RSK_SKIP_EPONA_RACE],
+        &mOptions[RSK_SKIP_TOWER_ESCAPE],
+        &mOptions[RSK_COMPLETE_MASK_QUEST],
+        &mOptions[RSK_SKIP_SCARECROWS_SONG]
+    }, false, WidgetContainerType::COLUMN);
+    mOptionGroups[RSG_ITEM_POOL_HINTS_IMGUI] = OptionGroup::SubGroup("", {
+        &mOptions[RSK_ITEM_POOL],
+        &mOptions[RSK_ICE_TRAPS],
+        &mOptions[RSK_GOSSIP_STONE_HINTS],
+        &mOptions[RSK_HINT_CLARITY],
+        &mOptions[RSK_HINT_DISTRIBUTION],
+    }, false, WidgetContainerType::SECTION);
+    mOptionGroups[RSG_EXTRA_HINTS_IMGUI] = OptionGroup::SubGroup("Extra Hints", {
+        &mOptions[RSK_TOT_ALTAR_HINT],
+        &mOptions[RSK_LIGHT_ARROWS_HINT],
+        &mOptions[RSK_DAMPES_DIARY_HINT],
+        &mOptions[RSK_GREG_HINT],
+        &mOptions[RSK_SARIA_HINT],
+        &mOptions[RSK_FROGS_HINT],
+        &mOptions[RSK_WARP_SONG_HINTS],
+        &mOptions[RSK_SCRUB_TEXT_HINT],
+        &mOptions[RSK_KAK_10_SKULLS_HINT],
+        &mOptions[RSK_KAK_20_SKULLS_HINT],
+        &mOptions[RSK_KAK_30_SKULLS_HINT],
+        &mOptions[RSK_KAK_40_SKULLS_HINT],
+        &mOptions[RSK_KAK_50_SKULLS_HINT]
+    }, false, WidgetContainerType::SECTION, "This setting adds some hints at locations other than Gossip Stones.\n\n"
+                    "House of Skulltula: # - Talking to a cursed House of Skulltula resident will tell you the reward"
+                    "they will give you for obtaining that many tokens.");
+    mOptionGroups[RSG_ITEM_POOL_HINTS_IMGUI_COLUMN] = OptionGroup::SubGroup("Item Pool & Hints", std::initializer_list<OptionGroup*>{
+        &mOptionGroups[RSG_ITEM_POOL_HINTS_IMGUI],
+        &mOptionGroups[RSG_EXTRA_HINTS_IMGUI],
+    }, false, WidgetContainerType::COLUMN);
+    mOptionGroups[RSG_ADDITIONAL_FEATURES_IMGUI] = OptionGroup::SubGroup("Additional Features", {
+        &mOptions[RSK_FULL_WALLETS],
+        &mOptions[RSK_BOMBCHUS_IN_LOGIC],
+        &mOptions[RSK_ENABLE_BOMBCHU_DROPS],
+        &mOptions[RSK_BLUE_FIRE_ARROWS],
+        &mOptions[RSK_SUNLIGHT_ARROWS]
+    }, false, WidgetContainerType::COLUMN);
+    mOptionGroups[RSG_GAMEPLAY_IMGUI_TABLE] = OptionGroup::SubGroup("Gameplay", {
+        &mOptionGroups[RSG_TIMESAVERS_IMGUI],
+        &mOptionGroups[RSG_ITEM_POOL_HINTS_IMGUI_COLUMN],
+        &mOptionGroups[RSG_ADDITIONAL_FEATURES_IMGUI]
+    }, false, WidgetContainerType::TABLE);
+    mOptionGroups[RSG_STARTING_EQUIPMENT_IMGUI] = OptionGroup::SubGroup("Starting Equipment", {
+        &mOptions[RSK_LINKS_POCKET],
+        &mOptions[RSK_STARTING_KOKIRI_SWORD],
+        &mOptions[RSK_STARTING_MASTER_SWORD],
+        &mOptions[RSK_STARTING_DEKU_SHIELD]
+    }, false, WidgetContainerType::COLUMN);
+    mOptionGroups[RSG_STARTING_ITEMS_IMGUI] = OptionGroup::SubGroup("Starting Items", {
+        &mOptions[RSK_STARTING_OCARINA],
+        &mOptions[RSK_STARTING_CONSUMABLES],
+        &mOptions[RSK_STARTING_SKULLTULA_TOKEN],
+    }, false, WidgetContainerType::COLUMN);
+    mOptionGroups[RSG_STARTING_NORMAL_SONGS_IMGUI] = OptionGroup::SubGroup("Normal Songs", {
+        &mOptions[RSK_STARTING_ZELDAS_LULLABY],
+        &mOptions[RSK_STARTING_EPONAS_SONG],
+        &mOptions[RSK_STARTING_SARIAS_SONG],
+        &mOptions[RSK_STARTING_SUNS_SONG],
+        &mOptions[RSK_STARTING_SONG_OF_TIME],
+        &mOptions[RSK_STARTING_SONG_OF_STORMS],
+    }, false, WidgetContainerType::SECTION);
+    mOptionGroups[RSG_STARTING_WARP_SONGS_IMGUI] = OptionGroup::SubGroup("Warp Songs", {
+        &mOptions[RSK_STARTING_MINUET_OF_FOREST],
+        &mOptions[RSK_STARTING_BOLERO_OF_FIRE],
+        &mOptions[RSK_STARTING_SERENADE_OF_WATER],
+        &mOptions[RSK_STARTING_REQUIEM_OF_SPIRIT],
+        &mOptions[RSK_STARTING_NOCTURNE_OF_SHADOW],
+        &mOptions[RSK_STARTING_PRELUDE_OF_LIGHT]
+    }, false, WidgetContainerType::SECTION);
+    mOptionGroups[RSG_STARTING_SONGS_IMGUI] = OptionGroup::SubGroup("Starting Songs", std::initializer_list<OptionGroup*>({
+        &mOptionGroups[RSG_STARTING_NORMAL_SONGS_IMGUI],
+        &mOptionGroups[RSG_STARTING_WARP_SONGS_IMGUI],
+    }), false, WidgetContainerType::COLUMN);
+    mOptionGroups[RSG_STARTING_INVENTORY_IMGUI_TABLE] = OptionGroup::SubGroup("Starting Inventory", {
+        &mOptionGroups[RSG_STARTING_EQUIPMENT_IMGUI],
+        &mOptionGroups[RSG_STARTING_ITEMS_IMGUI],
+        &mOptionGroups[RSG_STARTING_SONGS_IMGUI]
+    }, false, WidgetContainerType::TABLE);


See this comment about the duplication of OptionGroup contents. Removing the duplication would require lots of spoiler parsing changes.

leggettc18 · 2023-11-28T17:19:29Z

soh/soh/Enhancements/randomizer/option_descriptions.cpp

@@ -0,0 +1,523 @@
+#include "settings.h"


Bit of a tangent, obviously a much larger conversation, but should we consider moving this type of static string constant data to soh.otr or something? Might be easier to facilitate translations that way.

This is equivalent, the access through ctx in this case was completely unnecessary and a muscle-memory mistake on my part.

jbodner09 suggested changes Nov 13, 2023

View reviewed changes

leggettc18 added 15 commits November 17, 2023 13:10

Early version of ImGui Render function

c64bf4f

Currently only the slider variant. Will allow for auto rendering of options in ImGui, with tooltips and automatic display of the values in each Option's options array while keeping the CVars at the selected index, preventing Off By One Errors.

Implementation of Checkbox and Combobox rendering.

d6ee47c

Currently only in use for a couple of items, future commit will implement for all options.

Auto-render entire first tab of Randomizer Settings

e49fef6

Switch remaining tabs to auto-render

de53d95

Implements disabling options

eba7afd

Cleanup/Documentation

087ba09

Auto-render entire table columns

c4c06c5

Implement OptionGroup rendering for "Sections"

94d5405

Automates the rendering of tables in the Settings window.

63157fd

With the exception of the Locations and Tricks tabs, those are special and will need a lot more work.

Adds ability for option groups to have descriptions,

c726b83

These descriptions will automatically display as tooltips in ImGui, if the widget container type accounts for it.

Fixes some simple bugs

b3a571c

fix another small oopsie

cedc899

leggettc18 force-pushed the rando-settings-streamline branch from 98146cf to 15508c5 Compare November 17, 2023 18:11

Merge branch 'develop-rando' of https://github.com/HarbourMasters/Shi…

365be15

…pwright into rando-settings-streamline

leggettc18 marked this pull request as ready for review November 21, 2023 19:27

leggettc18 commented Nov 28, 2023

View reviewed changes

leggettc18 added 5 commits November 28, 2023 13:05

Fix bug with status of skip child stealth

abc89a8

Renames the Settings::Setting function to GetOption

4e1c11a

Add Settings pointer as a member of RandomizerSettingsWindow.

da37154

Replaces ctx->GetOption with direct access to mOptions

f4a9d72

This is equivalent, the access through ctx in this case was completely unnecessary and a muscle-memory mistake on my part.

Merge branch 'develop-rando' into rando-settings-streamline

a5bcfe7

leggettc18 added 3 commits December 5, 2023 19:36

Implements a few IDE/Linter suggestions

dded8eb

Merge branch 'develop-rando' into rando-settings-streamline

e2e1a44

Merge branch 'develop-rando' into rando-settings-streamline

130a390

leggettc18 merged commit dad4ae0 into HarbourMasters:develop-rando Dec 10, 2023

leggettc18 deleted the rando-settings-streamline branch December 10, 2023 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Rando settings streamline #3391

Rando settings streamline #3391

Uh oh!

leggettc18 commented Nov 13, 2023 •

edited by github-actions bot

Loading

Uh oh!

jbodner09 Nov 13, 2023

Uh oh!

leggettc18 Nov 17, 2023

Uh oh!

jbodner09 Nov 13, 2023

Uh oh!

leggettc18 Nov 17, 2023

Uh oh!

jbodner09 Nov 13, 2023

Uh oh!

leggettc18 Nov 17, 2023

Uh oh!

leggettc18 Nov 17, 2023

Uh oh!

leggettc18 commented Nov 17, 2023

Uh oh!

leggettc18 commented Nov 21, 2023

Uh oh!

leggettc18 Nov 28, 2023

Uh oh!

leggettc18 Nov 28, 2023

Uh oh!

leggettc18 Nov 28, 2023

Uh oh!

leggettc18 Nov 28, 2023

Uh oh!

leggettc18 Nov 28, 2023

Uh oh!

leggettc18 Nov 28, 2023

Uh oh!

Uh oh!

Rando settings streamline #3391

Rando settings streamline #3391

Uh oh!

Conversation

leggettc18 commented Nov 13, 2023 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Artifacts

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leggettc18 commented Nov 17, 2023

Uh oh!

leggettc18 commented Nov 21, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

leggettc18 commented Nov 13, 2023 •

edited by github-actions bot

Loading