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

WebHost: Expose name groups through the weighted-settings UI #2327

Merged
merged 7 commits into from
Oct 31, 2023

Conversation

nex3
Copy link
Contributor

@nex3 nex3 commented Oct 19, 2023

What is this fixing or adding?

It's already possible for worlds to define location_name_groups and
item_name_groups that users can use in their YAML files anywhere
that takes a location or item list, respectively. This PR exposes this
capability in the web UI as well, including the name groups as a
separate section at the beginning of item and location lists.

How was this tested?

Manually tested by defining groups in the Dark Souls III world,
generating YAML files, and so on.

If this makes graphical changes, please attach screenshots.

image

@ThePhar ThePhar added is: enhancement Issues requesting new features or pull requests implementing new features. affects: webhost Issues/PRs that touch webhost and may need additional validation. labels Oct 19, 2023
@LegendaryLinux LegendaryLinux self-assigned this Oct 25, 2023
@LegendaryLinux
Copy link
Member

So you manually created a group in the Dark Souls 3 world to test this? I checked out this PR and that group does not exist for me. I presume your manually created group was just for testing on your end, then?

@nex3
Copy link
Contributor Author

nex3 commented Oct 28, 2023

There's already one item group defined here, but I also merged in #2309 and tested with those groups as well.

@LegendaryLinux
Copy link
Member

Maybe I'm missing something. I've checked out your branch and confirmed the Dark Souls 3 item group exists in the branch in the world file, but the item group does not appear in my weighted-options page.

image

@nex3
Copy link
Contributor Author

nex3 commented Oct 28, 2023

That's so odd! I see it right here, running against an unmodified checkout of this commit:

image

I wonder if it's some kind of merge issue? Let me try merging the latest main.

nex3 and others added 4 commits October 27, 2023 21:18
The request for the JSON file that provides the setting data was missed during the rename in ArchipelagoMW#2037, so prior to this the weighted settings page wasn't rendering at all.
@nex3
Copy link
Contributor Author

nex3 commented Oct 31, 2023

Well, it looks like the latest main broke the weighted settings page, but #2408 should fix that. I've verified in a clean Codespace that this works as expected with that fix. Is there anything else you need from me here?

Copy link
Member

@LegendaryLinux LegendaryLinux left a comment

Choose a reason for hiding this comment

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

Was finally able to verify this. Approved.

@LegendaryLinux LegendaryLinux merged commit dc80f59 into ArchipelagoMW:main Oct 31, 2023
12 checks passed
@nex3 nex3 deleted the location-groups branch October 31, 2023 21:40
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
…lagoMW#2327)

* Factor out a common function for building lists

* Expose name groups through the weighted-settings UI

* Fix weighted-settings page

The request for the JSON file that provides the setting data was missed during the rename in ArchipelagoMW#2037, so prior to this the weighted settings page wasn't rendering at all.
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
…lagoMW#2327)

* Factor out a common function for building lists

* Expose name groups through the weighted-settings UI

* Fix weighted-settings page

The request for the JSON file that provides the setting data was missed during the rename in ArchipelagoMW#2037, so prior to this the weighted settings page wasn't rendering at all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: webhost Issues/PRs that touch webhost and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants