* Sweep events per-player to reduce sweep iterations
By finding all accessible locations per player and then collecting the
items from those locations, if any collected items belong to a different
player, then that player may be able to access more locations the next
time all of their accessible locations are found. This reduces the
number of iterations necessary to sweep through and collect from all
accessible locations.
* Also sweep per-player in MultiWorld.can_beat_game
* Deduplicate code by using sweep_for_events in can_beat_game
sweep_for_events has been modified to be able to return a generator and
to be able to change the set of locations that are filtered out. This
way, the same code can be used by both functions.
* Skip checking locations by assuming each world only logically depends on itself
While this assumption almost always holds true, worlds are allowed to
logically depend on other worlds, so the sweep always double checks at
the end by checking the locations of every world before finishing.
* Fix missed update to CollectionState.collect implementation
Collecting items with prevent_sweep=True (previously event=True) no
longer always returns True, so the return value should now be checked.
* Comment and variable name consistency/clarity
accessible/inaccessible -> reachable/unreachable
final sweep iteration -> extra sweep iteration
maybe_final_sweep -> checking_if_finished
* Apply suggestions from code review
Use Iterator in return type hint instead of Iterable to help indicate that the returned value can only be iterated once.
Be consistent in return statements. Because sweep_for_events can return a value now, the conditional branch that has no intended return value should explicitly return None.
Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>
* Update terminology from 'event' to 'advancement'
* Add typing overloads for sweep_for_advancements
This makes it so type-checkers and IDEs can see which calls return
`None` and which calls return `Iterator` so that it doesn't complain
about returning an `Iterator` from `sweep_for_events` or about iterating
through `None` in `can_beat_game`.
Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>
* Update comment for why discard the player after finding their locations
A lack of clarity was brought up in review.
* Update for removed typing import
---------
Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>
* Add new deprioritized item flag
* 4 retries
* indent
* .
* style
* I think this is nicer
* Nicer
* remove two lines again that I added unnecessarily
* I think this test makes a bit more sense like this
* Idk how to word this lol
* Add progression_deprioritized_skip_balancing bc why not ig
* More text
* Update Fill.py
* Update Fill.py
* I am the big stupid
* Actually collect the other half of progression items into state when filling without them
* More clarity on the descriptions (hopefully)
* visually separate technical description and use cases
* Actually make the call do what the comments say it does
Calling the dunder method has to:
1. Look up the dunder method for that object/class
2. Bind a new method instance to the object instance
3. Call the method with its arguments
4. Run the appropriate operation on the object
Whereas running the appropriate operation on the object from the start
skips straight to step 4.
Region.Register.__getitem__ is called a lot without #4583. In that case,
generation of 10 template Blasphemous yamls with
`--skip_output --seed 1` and progression balancing disabled went from
19.0s to 18.8s (1.3% reduction in generation duration).
From profiling with `timeit`
```py
def __getitem__(self, index: int) -> Location:
return self._list[index]
```
appears to be about twice as fast as the old code:
```py
def __getitem__(self, index: int) -> Location:
return self._list.__getitem__(index)
```
Besides this, there is not expected to be any noticeable difference in
performance, and there is not expected to be any difference in semantics
with these changes.
Co-authored-by: NewSoupVi <57900059+NewSoupVi@users.noreply.github.com>
* Fix playthrough
* oops
* oops 2
* I don't like this
* that should do it
* Update BaseClasses.py
Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>
* Update BaseClasses.py
---------
Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>
* region.add_event function
* Make it return the location bc why not
* Actually item bc that seems more useful
* Update BaseClasses.py
Co-authored-by: Aaron Wagener <mmmcheese158@gmail.com>
* Update BaseClasses.py
Co-authored-by: Aaron Wagener <mmmcheese158@gmail.com>
* add all the requested features from code review
* oop
* roughly sort args in order of importance (imo)
* Fix typing
---------
Co-authored-by: Aaron Wagener <mmmcheese158@gmail.com>
* Core: deprecate old options API
* also deprecate assigning options via option_definitions
---------
Co-authored-by: alwaysintreble <mmmcheese158@gmail.com>
`Location` does not override `__eq__` so should not override `__hash__`.
With this patch, this makes operations on sets of locations slightly
faster because they will use `object.__hash__` rather than
`Location.__hash__`.
`object.__hash__` is about 4 to 5 times faster than `Location.__hash__`
for me. Generation often uses sets of locations, so this slightly speeds
up generation.
The only place I could find that was hashing locations directly was
`WitnessLocationHint.__hash__`, but it has implemented a matching
`__eq__`, so is fine.
For security reasons, Python randomizes its hash seed each time it is
started, so the result of the `hash()` function is nondeterministic and
can't have been used by worlds for anything that needed to be
deterministic and can't have been used to compare information hashed at
generation time to information hashed by a client.
Without implementing __iter__ directly, calling iter() on a
Region.Register on Python 3.12 would return a new generator implemented
as follows:
```py
def __iter__(self) -> int:
i = 0
try:
while True:
v = self[i]
yield v
i += 1
except IndexError:
return None
```
This was determined by disassembling the returned generator with
dis.dis() and then constructing a function that disassembles into the
same bytecode.
The iterator returned by `iter(self._list)` is faster than this
generator, so using it slightly improves generation performance on
average.
Iteration of Region.Register objects is used a lot in
`CollectionState.update_reachable_regions` in both of the private
_update methods that get called. The performance gain here will vary
depending on how many regions a world has and how many exits those
regions have on average.
For a game like Blasphemous, with a lot of regions and exits, generation
of 10 template Blasphemous yamls with `--skip_output --seed 1` and
progression balancing disabled went from 19.0s to 16.4s (14.2% reduction
in generation duration).
* Core: Replace generator creation/iteration in CollectionState methods
Using generators in these functions incurs overhead to create the new
generator instance, call the `any`/`all`/`sum` function and have the
`any`/`all`/`sum` function iterate the generator, which in turn iterates
the iterable.
Replacing the use of generators with for loops is faster.
Getting `self.prog_items[player]` once in advance also improves
performance of iterating longer iterables.
* Add comment on the choice of for loops instead of any()/all()/sum()
* Initial implementation of Generic ER
* Move ERType to Entrance.Type, fix typing imports
* updates based on testing (read: flailing)
* Updates from feedback
* Various bug fixes in ERCollectionState
* Use deque instead of queue.Queue
* Allow partial entrances in collection state earlier, doc improvements
* Prevent early loops in region graph, improve reusability of ER stage code
* Typos, grammar, PEP8, and style "fixes"
* use RuntimeError instead of bare Exceptions
* return tuples from connect since it's slightly faster for our purposes
* move the shuffle to the beginning of find_pairing
* do er_state placements within pairing lookups to remove code duplication
* requested adjustments
* Add some temporary performance logging
* Use CollectionState to track available exits and placed regions
* Add a method to automatically disconnect entrances in a coupled-compliant way
Update docs and cleanup todos
* Make find_placeable_exits deterministic by sorting blocked_connections set
* Move EntranceType out of Entrance
* Handle minimal accessibility, autodetect regions, and improvements to disconnect
* Add on_connect callback to react to succeeded entrance placements
* Relax island-prevention constraints after a successful run on minimal accessibility; better error message on failure
* First set of unit tests for generic ER
* Change on_connect to send lists, add unit tests for EntranceLookup
* Fix duplicated location names in tests
* Update tests after merge
* Address review feedback, start docs with diagrams
* Fix rendering of hidden nodes in ER doc
* Move most docstring content into a docs article
* Clarify when randomize_entrances can be called safely
* Address review feedback
* Apply suggestions from code review
Co-authored-by: Aaron Wagener <mmmcheese158@gmail.com>
* Docs on ERPlacementState, add coupled/uncoupled handling to deadend detection
* Documentation clarifications
* Update groups to allow any hashable
* Restrict groups from hashable to int
* Implement speculative sweeping in stage 1, address misc review comments
* Clean unused imports in BaseClasses.py
* Restrictive region/speculative sweep test
* sweep_for_events->advancement
* Remove redundant __str__
Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>
* Allow partial entrances in auto indirect condition sweep
* Treat regions needed for logic as non-dead-end regardless of if they have exits, flip order of stage 3 and 4 to ensure there are enough exits for the dead ends
* Typing fixes suggested by mypy
* Remove erroneous newline
Not sure why the merge conflict editor is different and worse than the normal editor. Crazy
* Use modern typing for ER
* Enforce the use of explicit indirect conditions
* Improve doc on required indirect conditions
---------
Co-authored-by: qwint <qwint.42@gmail.com>
Co-authored-by: alwaysintreble <mmmcheese158@gmail.com>
Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>
* Core: Fix playthrough only checking half of the sphere 0 items
The lists of precollected items were being mutated while iterating those
same lists, causing playthrough to skip checking half of the sphere 0
advancement items.
This patch ensures the lists are copied before they are iterated.
* Replace chain.from_iterable with two for loops for better clarity
Added a comment to `multiworld.push_precollected(item)` to explain that
it is also modifying `precollected_items`.
Some worlds might want to check for "Item is junk", i.e. an excludable item.
Because this is both `filler` and `trap`, and because `filler` is `0`, there are many "wrong ways" to do this. So I think we should provide a helper function for it.
* Core: Move connection.parent_region assert to can_reach
This is how it already works for locations and it feels more correct to me to check in the place where the crash would happen.
Also update location error to be a bit more verbose
* Bring back the other assert
* Update BaseClasses.py
* fix single player item links
* Make a variable and fix weird spacing
* use advancement instead of classification
---------
Co-authored-by: Exempt-Medic <60412657+Exempt-Medic@users.noreply.github.com>
* Rename sweep_for_events to sweep_for_advancements
* more event->advancement renames
* oops accidentally deleted the deprecation thing in the force push
* Update TestDungeon.py
* Update BaseClasses.py
* Update BaseClasses.py
* oops
* utils.deprecate
* treble, you had no idea how right you were
* Update test_panel_hunt.py
* Update BaseClasses.py
Co-authored-by: Fabian Dill <Berserker66@users.noreply.github.com>
---------
Co-authored-by: Fabian Dill <Berserker66@users.noreply.github.com>
`MultiWorld.can_beat_game()` with no arguments would initially check if
`self.state` is beatable, but then would create an empty state,
`state = CollectionState(self)`, to sweep spheres from to determine if
the game is beatable. The issue was that `self.state` and the new empty
state could be different.
Currently, it seems that everywhere in Archipelago's codebase that calls
`MultiWorld.can_beat_game()` with no arguments or `starting_state=None`
has a `self.state` that only contains precollected items, so the new
empty state happens to result in an equivalent state, but this should
not be relied upon to always be the case.
This patch changes `can_beat_game()` to initially check if the new empty
state is beatable instead of `self.state`.
This appears to be a bug introduced way back in 27b6dd8bd7Fixes#3742
All the types being copied are built-in types with their own `copy()`
methods, so using the `copy` module was a bit overkill and also slower.
This patch replaces the use of the `copy` module in
`CollectionState.copy()` with using the built-in `.copy()` methods.
The copying of `reachable_regions` and `blocked_connections` was also
iterating the keys of each dictionary and then looking up the value in
the dictionary for that key. It is faster, and I think more readable, to
iterate the dictionary's `.items()` instead.
For me, when generating a multiworld including the template yaml of
every world with `python -O .\Generate.py --skip_output`, this patch
saves about 2.1s. The overall generation duration for these yamls varies
quite a lot, but averages around 160s for me, so on average this patch
reduced overall generation duration (excluding output duration) by
around 1.3%.
Timing comparisons were made by calling time.perf_counter() at the start
and end of `CollectionState.copy()`'s body, and summing the differences
between the starts and ends of the method body into a global variable
that was printed at the end of generation.
Additional timing comparisons were made, using the `timeit` module, of
the individual function calls or dictionary comprehensions used to
perform the copying.
The main performance cost was `copy.deepcopy()`, which gets slow as the
number of keys multiplied by the number of values within the
sets/Counters gets large, e.g., to deepcopy a `dict[int, Counter[str]]`
with 100 keys and where each Counter contains 100 keys was 30x slower
than most other tested copying methods. Increasing the number of dict
keys or Counter keys only makes it slower.
* Core: Check parent_region.can_reach first in Location.can_reach
The comment about self.access_rule computing faster on average appears
to no longer be correct with the current caching system for region
accessibility, resulting in self.parent_region.can_reach computing
faster on average.
Generation of template yamls for each game that does not require a rom
to generate, generated with `python -O .\Generate.py --seed 1`
(all durations averaged over at 4 or 5 generations):
Full generation with `spoiler: 1` and no progression balancing:
89.9s -> 72.6s
Only output from above case:
2.6s -> 2.2s
Full generation with `spoiler: 3` and no progression balancing:
769.9s -> 627.1s
Only playthrough calculation + paths from above case:
680.5s -> 555.3s
Full generation with `spoiler: 1` with default progression balancing:
123.5s -> 98.3s
Only progression balancing from above case:
11.3s -> 9.6s
* Update BaseClasses.py
* Update BaseClasses.py
* Update BaseClasses.py
---------
Co-authored-by: NewSoupVi <57900059+NewSoupVi@users.noreply.github.com>
* rename locations accessibility to "full" and make old locations accessibility debug only
* fix a bug in oot
* reorder lttp tests to not override its overrides
* changed the wrong word in the dict
* :forehead:
* update the manual lttp yaml
* use __debug__
* update pokemon and messenger
* fix conflicts from 993
* fix stardew presets
* add that locations may be inaccessible to description
* use reST format and make the items description one line so that it renders correctly on webhost
* forgot i renamed that
* add aliases for back compat
* some cleanup
* fix imports
* fix test failure
* only check "items" players when the item is progression
* Revert "only check "items" players when the item is progression"
This reverts commit ecbf986145e6194aa99a39c481d8ecd0736d5a4c.
* remove some unnecessary diffs
* CV64: Add ItemsAccessibility
* put items description at the bottom of the docstring since that's it's visual order
* :
* rename accessibility reference in pokemon rb dexsanity
* make the rendered tooltips look nicer
* Core: move item linking out of main
* add a test that item link option correctly validates
* remove unused fluff
---------
Co-authored-by: NewSoupVi <57900059+NewSoupVi@users.noreply.github.com>
