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

alttp: Add fill_slot_data function #1919

Merged

Conversation

StripesOO7
Copy link
Contributor

What is this fixing or adding?

Add fill_slot_data function.
Used by StripesOO7's pop-tracker pack to auto populate settings as convenience for the user

How was this tested?

Generated a bunch of seeds and played most of them with a smaller private group.

If this makes graphical changes, please attach screenshots.

N/A

Add fill_slot_data function. 
Used by StripesOO7's pop-tracker pack to auto populate settings as convenience for the user
@ThePhar ThePhar added the is: enhancement Issues requesting new features or pull requests implementing new features. label Jun 30, 2023
@ThePhar ThePhar requested a review from Berserker66 June 30, 2023 21:36
@el-u
Copy link
Collaborator

el-u commented Jun 30, 2023

Should this be disabled in race mode?

@Berserker66
Copy link
Member

Yes

@StripesOO7
Copy link
Contributor Author

Is that change somewhat close to what you both were thinking of as "disabling in race mode"?

@ThePhar
Copy link
Member

ThePhar commented Jul 22, 2023

@Berserker66 does this look alright with you now?

def fill_slot_data(self):
slot_data = {}
if not self.multiworld.is_race:
for option_name in alttp_options:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like that this is pulling every option in the options system. I know that isn't currently a lot for LTTP, but there's still some there that I doubt you need, like palette shuffle, and this will only get worse as the options get moved over.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like that this is pulling every option in the options system. I know that isn't currently a lot for LTTP, but there's still some there that I doubt you need, like palette shuffle, and this will only get worse as the options get moved over.

which version would you prefere then?
having a static list, iterating over it and adding it to slot_data?
OR simply having ~20 static, basically identical, lines for each option i need/want for the tracker?

Copy link
Collaborator

Choose a reason for hiding this comment

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

having a static list, iterating over it and adding it to slot_data?

This exactly. you make it sound like a bad thing? fwiw #993 makes this significantly easier, but we don't have that yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh. no. I have nothing against that solution and never had.

I will change that when i'm back home.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is that what you intended it look like?

…ing all alttp_options.

additional options needed to be done separately cause they are not stored the same way as the rest. "mode", "goal", etc. are simple values as the rest are key:value pairs so `.value` is not supported and I didn't want to introduce an if-statement.
…ing all alttp_options.

additional options needed to be done separately cause they are not stored the same way as the rest. "mode", "goal", etc. are simple values as the rest are key:value pairs so `.value` is not supported and I didn't want to introduce an if-statement.
…' into LTTP-add-fill_slot_data-function

# Conflicts:
#	worlds/alttp/__init__.py
Copy link
Collaborator

@alwaysintreble alwaysintreble left a comment

Choose a reason for hiding this comment

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

LGTM

@Berserker66
Copy link
Member

This should have a comment saying what it is for. As the default assumption for slot data would be that the client needs it, which is not the case at all.

@black-sliver black-sliver merged commit f29d5c8 into ArchipelagoMW:main Jul 30, 2023
@StripesOO7 StripesOO7 deleted the LTTP-add-fill_slot_data-function branch July 31, 2023 09:32
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
* __init__.py: Add fill_slot_data function

Add fill_slot_data function. 
Used by StripesOO7's pop-tracker pack to auto populate settings as convenience for the user

* LTTP__init__.py added race condition to fill_slot_data

* added missing self to multiworl.is_race

* changed filling of slot_data to fill from static list instead of pulling all alttp_options.
additional options needed to be done separately cause they are not stored the same way as the rest. "mode", "goal", etc. are simple values as the rest are key:value pairs so `.value` is not supported and I didn't want to introduce an if-statement.

* changed filling of slot_data to fill from static list instead of pulling all alttp_options.
additional options needed to be done separately cause they are not stored the same way as the rest. "mode", "goal", etc. are simple values as the rest are key:value pairs so `.value` is not supported and I didn't want to introduce an if-statement.

* added a comment to describe the use for the option added to slot_data

---------

Co-authored-by: StripesOO7 <[email protected]>
kl3cks7r pushed a commit to kl3cks7r/Archipelago that referenced this pull request Dec 15, 2023
* __init__.py: Add fill_slot_data function

Add fill_slot_data function. 
Used by StripesOO7's pop-tracker pack to auto populate settings as convenience for the user

* LTTP__init__.py added race condition to fill_slot_data

* added missing self to multiworl.is_race

* changed filling of slot_data to fill from static list instead of pulling all alttp_options.
additional options needed to be done separately cause they are not stored the same way as the rest. "mode", "goal", etc. are simple values as the rest are key:value pairs so `.value` is not supported and I didn't want to introduce an if-statement.

* changed filling of slot_data to fill from static list instead of pulling all alttp_options.
additional options needed to be done separately cause they are not stored the same way as the rest. "mode", "goal", etc. are simple values as the rest are key:value pairs so `.value` is not supported and I didn't want to introduce an if-statement.

* added a comment to describe the use for the option added to slot_data

---------

Co-authored-by: StripesOO7 <[email protected]>
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
* __init__.py: Add fill_slot_data function

Add fill_slot_data function. 
Used by StripesOO7's pop-tracker pack to auto populate settings as convenience for the user

* LTTP__init__.py added race condition to fill_slot_data

* added missing self to multiworl.is_race

* changed filling of slot_data to fill from static list instead of pulling all alttp_options.
additional options needed to be done separately cause they are not stored the same way as the rest. "mode", "goal", etc. are simple values as the rest are key:value pairs so `.value` is not supported and I didn't want to introduce an if-statement.

* changed filling of slot_data to fill from static list instead of pulling all alttp_options.
additional options needed to be done separately cause they are not stored the same way as the rest. "mode", "goal", etc. are simple values as the rest are key:value pairs so `.value` is not supported and I didn't want to introduce an if-statement.

* added a comment to describe the use for the option added to slot_data

---------

Co-authored-by: StripesOO7 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

6 participants