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

TUNIC: Use fewer parameters in helper functions #3356

Merged
merged 13 commits into from
Jul 5, 2024

Conversation

ScipioWright
Copy link
Collaborator

@ScipioWright ScipioWright commented May 21, 2024

What is this fixing or adding?

Tunic had a bunch of ruler helper functions with way too many parameters.
Now they don't!

How was this tested?

Ran gen a few times, zipped it into an apworld and ran gen on frozen just in case

If this makes graphical changes, please attach screenshots.

N/A

@ScipioWright ScipioWright added the is: refactor/cleanup Improvements to code/output readability or organizization. label May 21, 2024
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label May 21, 2024
@ScipioWright ScipioWright changed the title TUNIC: Use less parameters in helper functions by abusing state.multiworld TUNIC: Use less parameters in helper functions by "abusing" state.multiworld May 21, 2024
@NewSoupVi
Copy link
Member

NewSoupVi commented Jun 1, 2024

For what it's worth, I definitely would have preferred passing world and getting player from world.player and multiworld from world.multiworld

It's the same number of args (state, world vs state, player) but the relationships make a bit more sense imo, and you don't have to do any goofy casting. The world is the central object to a slot and it's what the player, the options, etc. "come from".

Ftr, I don't think there is anything technically wrong with what you're doing here though.

@ScipioWright ScipioWright changed the title TUNIC: Use less parameters in helper functions by "abusing" state.multiworld TUNIC: Use less parameters in helper functions Jun 1, 2024
Copy link
Contributor

@EmilyV99 EmilyV99 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@agilbert1412 agilbert1412 left a comment

Choose a reason for hiding this comment

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

Changes seem to be the same thing over and over a million times. And that thing seems like a good one. So approved

@ScipioWright ScipioWright 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 Jun 10, 2024
@ScipioWright ScipioWright changed the title TUNIC: Use less parameters in helper functions TUNIC: Use fewer parameters in helper functions Jun 27, 2024
@NewSoupVi NewSoupVi merged commit e7a8e19 into ArchipelagoMW:main Jul 5, 2024
17 checks passed
@ScipioWright ScipioWright deleted the tunc-cleanup-rules branch July 5, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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