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

Timespinner: support new flags and settings from the randomizer #4559

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

sgrunt
Copy link
Contributor

@sgrunt sgrunt commented Jan 27, 2025

What is this fixing or adding?

Several flags and settings have been added to recent releases of the Timespinner randomiser; this pull requests updates the apworld to support those flags and settings. There is a companion pull request for the randomiser itself, Jarno458/TsRandomizer#164, to properly support a couple of the new flags and settings here.

How was this tested?

Rolled several seeds locally to check logic. Ran webhost to verify updates to the tracker. Played through a webhosted seed to verify general functionality (and tracker updates).

If this makes graphical changes, please attach screenshots.

image

@github-actions github-actions bot added affects: webhost Issues/PRs that touch webhost and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Jan 27, 2025
}
#inventory-table img.acquired.crimson{ /*DB143B*/
filter: hue-rotate(348deg) saturate(8.3) brightness(0.47);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These three colours are the same as those used by the randomiser's item tracker for the three Laser Access items.

if self.PyramidStart != PyramidStart.default and \
self.pyramid_start == PyramidStart.default:
self.pyramid_start.value = self.PyramidStart.value
self.has_replaced_options.value = Toggle.option_true
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'm not sold on the idea of supporting the old casing for the new settings, but decided to put them in for posterity.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll let Jarno decide on this, but I'm pretty sure I heard them say in the past that there's no need to add new options to the backwards compat stuff

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah don't put new options in backward compatibility, if people want to use the new options they might notice the casing difference and be more eager to update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gone now.


if options.inverted:
if options.inverted or (options.pyramid_start and not options.back_to_the_future):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strictly speaking this doesn't match the in-randomiser logic (which will allow both past and present for pyramid start mode based on my understanding), but generation tends to fail out if I don't restrict this case to present gates.

worlds/timespinner/Regions.py Outdated Show resolved Hide resolved
@@ -125,7 +125,7 @@ def create_regions_and_locations(world: MultiWorld, player: int, options: Timesp
connect(world, player, 'Sealed Caves (Xarion)', 'Skeleton Shaft')
connect(world, player, 'Sealed Caves (Xarion)', 'Space time continuum', logic.has_teleport)
connect(world, player, 'Refugee Camp', 'Forest')
connect(world, player, 'Refugee Camp', 'Library', lambda state: options.inverted and options.back_to_the_future and state.has_all({'Timespinner Wheel', 'Timespinner Spindle'}, player))
connect(world, player, 'Refugee Camp', 'Library', lambda state: (options.pyramid_start or options.inverted) and options.back_to_the_future and state.has_all({'Timespinner Wheel', 'Timespinner Spindle'}, player))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In-rando logic checks for "not present start", encompassing both pyramid start and inverted.

@@ -162,9 +162,12 @@ def create_regions_and_locations(world: MultiWorld, player: int, options: Timesp
connect(world, player, 'Royal towers (upper)', 'Royal towers')
#connect(world, player, 'Ancient Pyramid (entrance)', 'The lab (upper)', lambda state: not is_option_enabled(world, player, "EnterSandman"))
connect(world, player, 'Ancient Pyramid (entrance)', 'Ancient Pyramid (left)', logic.has_doublejump)
connect(world, player, 'Ancient Pyramid (entrance)', 'Space time continuum', logic.has_teleport)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are here to support pyramid start.

@@ -376,7 +388,8 @@ def assign_starter_item(self, excluded_items: Set[str], location: str, item_list
self.place_locked_item(excluded_items, location, item_name)

def place_first_progression_item(self, excluded_items: Set[str]) -> None:
if self.options.quick_seed or self.options.inverted or self.precalculated_weights.flood_lake_desolation:
if (self.options.quick_seed or self.options.inverted or self.precalculated_weights.flood_lake_desolation) \
and not self.options.pyramid_start:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enabling the early progression placement here to allow pyramid start seeds to generate faster (there are all of three checks open initially).

@ScipioWright ScipioWright added waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world. is: enhancement Issues requesting new features or pull requests implementing new features. labels Jan 27, 2025
@ScipioWright ScipioWright requested a review from Jarno458 January 27, 2025 14:12
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.

Only did code review, I do not know the game so I cannot comment on the logic

worlds/timespinner/Locations.py Outdated Show resolved Hide resolved
if self.PyramidStart != PyramidStart.default and \
self.pyramid_start == PyramidStart.default:
self.pyramid_start.value = self.PyramidStart.value
self.has_replaced_options.value = Toggle.option_true
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll let Jarno decide on this, but I'm pretty sure I heard them say in the past that there's no need to add new options to the backwards compat stuff

LocationData('The lab (power off)', 'Lab: Middle terminal (Amadeus Laboratory Map)', 1337166, lambda state: state.has('Tablet', player)),
LocationData('The lab (power off)', 'Lab: Sentry platform terminal (Origins)', 1337167, lambda state: state.has('Tablet', player)),
LocationData('The lab (power off)', 'Lab: Middle terminal (Amadeus Laboratory Map)', 1337166, lambda state: state.has('Tablet', player) and (not options.lock_key_amadeus or state.has('Lab Access Research', player))),
LocationData('The lab (power off)', 'Lab: Sentry platform terminal (Origins)', 1337167, lambda state: state.has('Tablet', player) and (not options.lock_key_amadeus or state.has('Lab Access Research', player))),
Copy link
Contributor Author

@sgrunt sgrunt Jan 27, 2025

Choose a reason for hiding this comment

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

  • This matches the upstream logic, but I think this is incorrect - the room this is in is gated behind Lab Access Genza, not Lab Access Research. Check upstream and update the other PR too if needed.

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've changed this here, but I need to verify that the problem is what I think it is and also fix in the main rando if so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rando PR has been updated with the fix.

if self.PyramidStart != PyramidStart.default and \
self.pyramid_start == PyramidStart.default:
self.pyramid_start.value = self.PyramidStart.value
self.has_replaced_options.value = Toggle.option_true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah don't put new options in backward compatibility, if people want to use the new options they might notice the casing difference and be more eager to update

{% endif %}
{% if 'GateKeep' in options %}
<div class="C3">
<span class="{{ 'acquired' if 'Drawbridge Key' in acquired_items }}" title="Lab Access Experiment">&#10070;</span>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There didn't seem to be a suitable icon for the sprite I used for the Drawbridge Key available from the source of the rest of the icons, so I'm emulating it with a ❖ character instead. The collected version looks like this:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: webhost Issues/PRs that touch webhost and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants