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

Fix options pages sometimes displaying blank values in form fields #3364

Merged
merged 2 commits into from
Jun 1, 2024

Conversation

LegendaryLinux
Copy link
Member

What is this fixing or adding?

This fixes a bug causing values on the options pages to sometimes display as blank fields if a preset was chosen which was defined in a particular way

How was this tested?

Tested locally by running WebHost

If this makes graphical changes, please attach screenshots.

Before:

image

After:

image

@LegendaryLinux LegendaryLinux self-assigned this May 21, 2024
@github-actions github-actions bot added affects: webhost Issues/PRs that touch webhost and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels May 21, 2024
@LegendaryLinux LegendaryLinux added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. affects: release/blocker Issues/PRs that must be addressed before next official release. labels May 21, 2024
@Ishigh1
Copy link
Contributor

Ishigh1 commented May 21, 2024

I manually checked all presets, they all look correct now

Comment on lines +85 to +109
presets[preset_name] = {}
for preset_option_name, preset_option in preset.items():
if preset_option == "random":
presets[preset_name][preset_option_name] = preset_option
continue

option = world.options_dataclass.type_hints[preset_option_name].from_any(preset_option)
if isinstance(option, Options.NamedRange) and isinstance(preset_option, str):
assert preset_option in option.special_range_names, \
f"Invalid preset value '{preset_option}' for '{preset_option_name}' in '{preset_name}'. " \
f"Expected {option.special_range_names.keys()} or {option.range_start}-{option.range_end}."

presets[preset_name][preset_option_name] = option.value
elif isinstance(option, Options.Range):
presets[preset_name][preset_option_name] = option.value
elif isinstance(preset_option, str):
# Ensure the option value is valid for Choice and Toggle options
assert option.name_lookup[option.value] == preset_option, \
f"Invalid option value '{preset_option}' for '{preset_option_name}' in preset '{preset_name}'. " \
f"Values must not be resolved to a different option via option.from_text (or an alias)."
# Use the name of the option
presets[preset_name][preset_option_name] = option.current_key
else:
# Use the name of the option
presets[preset_name][preset_option_name] = option.current_key
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
presets[preset_name] = {}
for preset_option_name, preset_option in preset.items():
if preset_option == "random":
presets[preset_name][preset_option_name] = preset_option
continue
option = world.options_dataclass.type_hints[preset_option_name].from_any(preset_option)
if isinstance(option, Options.NamedRange) and isinstance(preset_option, str):
assert preset_option in option.special_range_names, \
f"Invalid preset value '{preset_option}' for '{preset_option_name}' in '{preset_name}'. " \
f"Expected {option.special_range_names.keys()} or {option.range_start}-{option.range_end}."
presets[preset_name][preset_option_name] = option.value
elif isinstance(option, Options.Range):
presets[preset_name][preset_option_name] = option.value
elif isinstance(preset_option, str):
# Ensure the option value is valid for Choice and Toggle options
assert option.name_lookup[option.value] == preset_option, \
f"Invalid option value '{preset_option}' for '{preset_option_name}' in preset '{preset_name}'. " \
f"Values must not be resolved to a different option via option.from_text (or an alias)."
# Use the name of the option
presets[preset_name][preset_option_name] = option.current_key
else:
# Use the name of the option
presets[preset_name][preset_option_name] = option.current_key
current_preset = presets[preset_name] = {}
for preset_option_name, preset_option in preset.items():
if preset_option == "random":
current_preset[preset_option_name] = preset_option
continue
option = world.options_dataclass.type_hints[preset_option_name].from_any(preset_option)
if isinstance(option, Options.NamedRange) and isinstance(preset_option, str):
assert preset_option in option.special_range_names, \
f"Invalid preset value '{preset_option}' for '{preset_option_name}' in '{preset_name}'. " \
f"Expected {option.special_range_names.keys()} or {option.range_start}-{option.range_end}."
current_preset[preset_option_name] = option.value
elif isinstance(option, Options.Range):
current_preset[preset_option_name] = option.value
elif isinstance(preset_option, str):
# Ensure the option value is valid for Choice and Toggle options
assert option.name_lookup[option.value] == preset_option, \
f"Invalid option value '{preset_option}' for '{preset_option_name}' in preset '{preset_name}'. " \
f"Values must not be resolved to a different option via option.from_text (or an alias)."
# Use the name of the option
current_preset[preset_option_name] = option.current_key
else:
# Use the name of the option
current_preset[preset_option_name] = option.current_key

Minor thing for minor readability and execution speed improvement.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this causing current_preset to always equal the value of presets[preset_name]?

@LegendaryLinux LegendaryLinux merged commit f3003ff into main Jun 1, 2024
26 checks passed
@LegendaryLinux LegendaryLinux deleted the fix-option-presets branch June 1, 2024 02:41
jnschurig pushed a commit to Tranquilite0/Archipelago-SoulBlazer that referenced this pull request Jun 13, 2024
sflavelle pushed a commit to sflavelle/Archipelago-tgc that referenced this pull request Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: release/blocker Issues/PRs that must be addressed before next official release. affects: webhost Issues/PRs that touch webhost and may need additional validation. is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants