Skip to content

Converting enum-like string settings to proper enums (attempt #3)#1306

Merged
Killklli merged 27 commits into
2dos:devfrom
TheSoundDefense:settings-to-enums
Feb 21, 2023
Merged

Converting enum-like string settings to proper enums (attempt #3)#1306
Killklli merged 27 commits into
2dos:devfrom
TheSoundDefense:settings-to-enums

Conversation

@TheSoundDefense
Copy link
Copy Markdown
Collaborator

Original PR description:

This pull request takes all of the web page settings that acted as enums, but used string values ("move_rando", "win_condition", etc.), and replaces them with explicit enums. Other settings in the main Settings object have also been replaced with enums for safety. (Incorrect strings will fail silently, but incorrect enums will crash and direct users to the bug in the code.)

All of the new enums have been placed in randomizer.Enums.Settings. This file also includes a SettingsMap dictionary, to allow for easy conversion from strings to enums. For example: calling SettingsMap["damage_amount"]["ohko"] returns DamageAmount.ohko, and SettingsMap["starting_keys_list_selected"]["AngryAztecKey"] returns Items.AngryAztecKey. Conversion from ints to enums also works: calling SettingsMap["damage_amount"](2) returns DamageAmount.ohko.

Four of the enums (ActivateAllBananaports, WinCondition, HelmSetting, and MicrohintsEnabled) have been explicitly indexed, starting from zero, so they can be directly written to the ROM in ApplyRandomizer.py.

The KeySelector and EnemySelector have been changed, so that the value is the string name of the corresponding enum. This simplifies the code and automates the enum conversion process.

This PR also identified a small bug in MoveLocationRando.py, where "start_with" was accidentally written as "starts_with". (Another good reason to favor enums.)

I was able to generate a seed with a legible spoiler log using this code, which hopefully means it works correctly.

Updates (attempt #2):

This new PR should fix the enum-conversion issue that existed when using cli.py. That file has been changed. A failsafe was also added into Settings.py, so if a string gets passed in place of an enum value, the code will convert that string to the enum if possible. Both of these fixes work independently of each other, as well as together.

Other changes.

  • Removed some calls to dumps in Fill.py, as they crashed the code and they were obsolete anyway.
  • Added boulder_clips to the GlitchesSelected enum, in case it becomes re-enabled at some point.
  • Changed from strings to the RandomPrices enum in Prices.py, which I previously missed.
  • Fixed "moonkick" -> "moonkicks" in Logic.py
  • Fixed "skipped" -> "skip' in ShuffleBarrels.py
  • Fixed "starts_with" -> "start_with" in PriceRando.py

Updates (attempt #3):

Fixed cli.py so that it can accept both enum and string values for the relevant settings. (I had previously assumed that only strings would be passed through cli.py, based on presets.)

TheSoundDefense and others added 27 commits February 15, 2023 23:01
Frontend work should hopefully be done
Also relocating the Settings enums to prevent a circular reference
Also renaming the Settings enum to FormSettings
Not really sure what I was thinking, turning the keys into enums is overkill for sore
Changing imported JSON data to use enums for the test
Why does MoveRando.item_shuffle exist? Double-check to see why.
Rearranging a couple of enums so default values are first.
Another minor lint issue
Simplifying the import into Settings.py
Added conversion from strings to enums in cli.py, and a failsafe in Settings.py.

Additional fixes:
- Removed dumps in Fill.py
- Changing from strings to enums in Prices.py (which I previously missed)
- Added boulder_clips to GlitchesSelected
- Fixed "moonkick" -> "moonkicks" in Logic.py
- Fixed "skipped" -> "skip" in ShuffleBarrels.py
- Fixed "starts_with" -> "start_with" in PriceRando.py
Covering the last item in the Settings object
Adjust cli.py so that it can accept data already converted to enums
Minor typo
@Killklli Killklli merged commit 2778089 into 2dos:dev Feb 21, 2023
@TheSoundDefense TheSoundDefense deleted the settings-to-enums branch February 28, 2023 02:59
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.

2 participants