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

SM and SMZ3 option_definitions deprecation fix #3372

Merged

Conversation

lordlou
Copy link
Contributor

@lordlou lordlou commented May 22, 2024

What is this fixing or adding?

Replaced deprecated use of option_definitions for SM and SMZ3 by options_dataclass

How was this tested?

Generated a multiworld with both game that succeeded without having warning message about deprecated use of option_definitions.

lordlou added 30 commits August 9, 2021 23:32
- fixed seeded generation
- fixed broken logic when more than one SM world
- added missing rules for inter-area transitions
- added basic patch presence for logic
- added DoorManager init call to reflect present patches for logic
- moved CollectionState addition out of BaseClasses into SM world
- added condition to apply progitempool presorting only if SM world is present
- set Bosses item id to None to prevent them going into multidata
- now use get_game_players
…VariaRandomizer's rules, items, locations and accessPoint (as regions)
…se (fills ROM item table)

This is dependant on modifications done to sm_randomizer_rom project
- player name inject in ROM and get in client
- end game get from ROM in client
- send self item to server
- add player names table in ROM
…ition of an array of it (required to generation more than one SM world, which is still fails but is better)
- fixed seeded generation
- fixed broken logic when more than one SM world
- added missing rules for inter-area transitions
- added basic patch presence for logic
- added DoorManager init call to reflect present patches for logic
- moved CollectionState addition out of BaseClasses into SM world
- added condition to apply progitempool presorting only if SM world is present
- set Bosses item id to None to prevent them going into multidata
- now use get_game_players
Copy link
Member

@Exempt-Medic Exempt-Medic left a comment

Choose a reason for hiding this comment

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

Changes LGTM. Generations (Ten sets of 100 fully random yamls) before and after were identical. No warning about deprecated option getters, and searches for others came up empty

@Exempt-Medic Exempt-Medic added the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Jun 4, 2024
@Exempt-Medic Exempt-Medic removed the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Jun 8, 2024
Copy link
Contributor

@qwint qwint left a comment

Choose a reason for hiding this comment

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

format of the new options API looks good, did some searching and did not find any other uses of the old metadata.option_name[player] format
generated with default options and skip_output and confirmed there was no sm/smz3 warnings about Options API
did not test option combinations or fully patching and playing the relevant games

@Exempt-Medic Exempt-Medic added waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Jul 30, 2024
@Exempt-Medic
Copy link
Member

This has conflicts btw @lordlou

@NewSoupVi NewSoupVi added the waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world. label Aug 11, 2024
@Exempt-Medic Exempt-Medic added waiting-on: author Issue/PR is waiting for feedback or changes from its author. and removed waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world. labels Aug 14, 2024
lordlou and others added 2 commits August 17, 2024 14:38
Copy link
Member

@Exempt-Medic Exempt-Medic left a comment

Choose a reason for hiding this comment

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

Reaffirming approval after the recent changes

@Exempt-Medic Exempt-Medic removed the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Aug 17, 2024
@NewSoupVi NewSoupVi merged commit 7e0219c into ArchipelagoMW:main Aug 31, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: maintenance Regular updates to requirements and utilities that do not fix bugs or change/add features. is: refactor/cleanup Improvements to code/output readability or organizization. waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants