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

The Witness: The big dumb refactor #3007

Merged
merged 15 commits into from
Apr 11, 2024
Merged

Conversation

NewSoupVi
Copy link
Member

@NewSoupVi NewSoupVi commented Mar 22, 2024

The structure of The Witness is some actual hot trash. Also it violates style a lot. Also it uses old APIs.

  • Ran ruff --fix
  • Made "static classes" into modules because Jesus Fcking Christ
  • Changed some small things here and there
  • Got rid of dumb naming like regio and locat
  • Cried at the thought of all the conflicts I will have to fix soon

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Mar 22, 2024
@NewSoupVi NewSoupVi added the is: refactor/cleanup Improvements to code/output readability or organizization. label Mar 22, 2024
@NewSoupVi
Copy link
Member Author

NewSoupVi commented Mar 22, 2024

For any potential reviewers: There is 0 new code here apart from new import statements. Everything has just been shifted around, reformatted, or moved to new files / different folders.
Don't waste a bunch of time reviewing functionality

@NewSoupVi
Copy link
Member Author

Included the ruff.toml that was used for this.

Made sure it actually passes, except for one thing that will be addressed by this PR: #3049

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.

Carefully read through all the changes, merged into main and generated five hundred 10-player games and one thousand single- and two-player games. Compared 0.4.5 and refactor spoiler logs (they were same except for playthrough) and walked through the playthrough checking for errors.

Copy link
Collaborator

@ScipioWright ScipioWright left a comment

Choose a reason for hiding this comment

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

Read through changes, they sure are changes. Did a bunch of test gens with 50 player worlds with some triggers to avoid that exception regarding low progression counts.

worlds/witness/__init__.py Outdated Show resolved Hide resolved
@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 Apr 5, 2024
Co-authored-by: Scipio Wright <scipiowright@gmail.com>
@Berserker66 Berserker66 merged commit 401a6d9 into ArchipelagoMW:main Apr 11, 2024
15 checks passed
EmilyV99 pushed a commit to EmilyV99/Archipelago that referenced this pull request Apr 15, 2024
EmilyV99 pushed a commit to EmilyV99/Archipelago that referenced this pull request Apr 15, 2024
qwint pushed a commit to qwint/Archipelago that referenced this pull request Jun 24, 2024
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