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

Various small additions #59

Merged
merged 12 commits into from
Jul 11, 2024
Merged

Conversation

wossnameGitHub
Copy link
Contributor

At first this may seem like a complicated Merge Request, but if you check the commits individually I think you'll quickly realise these are very simple changes.

  • changed order of some code blocks in entrance_rando.py
  • changed order of some code blocks in utils.py
  • the 2 highjack_transitions functions are moved from entrance_rando.py to utils.py
  • temp_disabled_exits is added (mainly used in the graph)
  • VALLEY_OF_THE_SPIRITS renamed to VALLEY_OF_SPIRITS
  • adjusted some of the spacing in the generated graphml file

Copy link
Owner

@Avasam Avasam left a comment

Choose a reason for hiding this comment

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

I'll finish this review and give a hand with static typing when I get back home.

But I'm not sure why so much stuff needs to be moved around. Does it even reduce forward references (making it so methods are declared before they're used visually)? It makes the git history messier, harder to find and track why things were added/removed and by whom (by using the "blame" tool. Also easily creates conflicts.

Otherwise the functional changes seem fine. The graph changes are a good improvement.

@Avasam Avasam merged commit f494c8a into Avasam:main Jul 11, 2024
3 checks passed
@wossnameGitHub wossnameGitHub deleted the various_small_additions branch July 11, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants