Fill: fix fill_restrictive for mixed minimal and non-minimal and test (#1800)

* Tests: add test for mixing minimal and non-minimal

* Tests: minor cleanup in test_minimal_mixed_fill

* fix fill_restrictive for mixed minimal/non-minimal

The reason why this only happens for minimal is because it would not accept the solution it found otherwise.
Tracking and releasing unreachable items would be the better solution, but that's a lot harder to do.

* fix typo in fill_restrictive

* fix pep8 in fill_restrictive

* Fill: cleanup invalid unsafe placements, better comments

* Fill: more cleanup
This commit is contained in:
black-sliver 2023-06-25 02:55:13 +02:00 committed by GitHub
parent 46f2f3d7cd
commit a45e8730cb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 62 additions and 11 deletions

38
Fill.py
View File

@ -39,8 +39,9 @@ def fill_restrictive(world: MultiWorld, base_state: CollectionState, locations:
"""
unplaced_items: typing.List[Item] = []
placements: typing.List[Location] = []
cleanup_required = False
swapped_items: typing.Counter[typing.Tuple[int, str]] = Counter()
swapped_items: typing.Counter[typing.Tuple[int, str, bool]] = Counter()
reachable_items: typing.Dict[int, typing.Deque[Item]] = {}
for item in item_pool:
reachable_items.setdefault(item.player, deque()).append(item)
@ -84,25 +85,28 @@ def fill_restrictive(world: MultiWorld, base_state: CollectionState, locations:
else:
# we filled all reachable spots.
if swap:
# try swapping this item with previously placed items
for (i, location) in enumerate(placements):
# try swapping this item with previously placed items in a safe way then in an unsafe way
swap_attempts = ((i, location, unsafe)
for unsafe in (False, True)
for i, location in enumerate(placements))
for (i, location, unsafe) in swap_attempts:
placed_item = location.item
# Unplaceable items can sometimes be swapped infinitely. Limit the
# number of times we will swap an individual item to prevent this
swap_count = swapped_items[placed_item.player,
placed_item.name]
swap_count = swapped_items[placed_item.player, placed_item.name, unsafe]
if swap_count > 1:
continue
location.item = None
placed_item.location = None
swap_state = sweep_from_pool(base_state, [placed_item])
# swap_state assumes we can collect placed item before item_to_place
swap_state = sweep_from_pool(base_state, [placed_item] if unsafe else [])
# unsafe means swap_state assumes we can somehow collect placed_item before item_to_place
# by continuing to swap, which is not guaranteed. This is unsafe because there is no mechanic
# to clean that up later, so there is a chance generation fails.
if (not single_player_placement or location.player == item_to_place.player) \
and location.can_fill(swap_state, item_to_place, perform_access_check):
# Verify that placing this item won't reduce available locations, which could happen with rules
# that want to not have both items. Left in until removal is proven useful.
# Verify placing this item won't reduce available locations, which would be a useless swap.
prev_state = swap_state.copy()
prev_loc_count = len(
world.get_reachable_locations(prev_state))
@ -117,13 +121,15 @@ def fill_restrictive(world: MultiWorld, base_state: CollectionState, locations:
spot_to_fill = placements.pop(i)
swap_count += 1
swapped_items[placed_item.player,
placed_item.name] = swap_count
swapped_items[placed_item.player, placed_item.name, unsafe] = swap_count
reachable_items[placed_item.player].appendleft(
placed_item)
item_pool.append(placed_item)
# cleanup at the end to hopefully get better errors
cleanup_required = True
break
# Item can't be placed here, restore original item
@ -144,6 +150,16 @@ def fill_restrictive(world: MultiWorld, base_state: CollectionState, locations:
if on_place:
on_place(spot_to_fill)
if cleanup_required:
# validate all placements and remove invalid ones
for placement in placements:
state = sweep_from_pool(base_state, [])
if world.accessibility[placement.item.player] != "minimal" and not placement.can_reach(state):
placement.item.location = None
unplaced_items.append(placement.item)
placement.item = None
locations.append(placement)
if allow_excluded:
# check if partial fill is the result of excluded locations, in which case retry
excluded_locations = [

View File

@ -199,6 +199,41 @@ class TestFillRestrictive(unittest.TestCase):
# Unnecessary unreachable Item
self.assertEqual(locations[1].item, items[0])
def test_minimal_mixed_fill(self):
"""
Test that fill for 1 minimal and 1 non-minimal player will correctly place items in a way that lets
the non-minimal player get all items.
"""
multi_world = generate_multi_world(2)
player1 = generate_player_data(multi_world, 1, 3, 3)
player2 = generate_player_data(multi_world, 2, 3, 3)
multi_world.accessibility[player1.id].value = multi_world.accessibility[player1.id].option_minimal
multi_world.accessibility[player2.id].value = multi_world.accessibility[player2.id].option_locations
multi_world.completion_condition[player1.id] = lambda state: True
multi_world.completion_condition[player2.id] = lambda state: state.has(player2.prog_items[2].name, player2.id)
set_rule(player1.locations[1], lambda state: state.has(player1.prog_items[0].name, player1.id))
set_rule(player1.locations[2], lambda state: state.has(player1.prog_items[1].name, player1.id))
set_rule(player2.locations[1], lambda state: state.has(player2.prog_items[0].name, player2.id))
set_rule(player2.locations[2], lambda state: state.has(player2.prog_items[1].name, player2.id))
# force-place an item that makes it impossible to have all locations accessible
player1.locations[0].place_locked_item(player1.prog_items[2])
# fill remaining locations with remaining items
location_pool = player1.locations[1:] + player2.locations
item_pool = player1.prog_items[:-1] + player2.prog_items
fill_restrictive(multi_world, multi_world.state, location_pool, item_pool)
multi_world.state.sweep_for_events() # collect everything
# all of player2's locations and items should be accessible (not all of player1's)
for item in player2.prog_items:
self.assertTrue(multi_world.state.has(item.name, player2.id),
f'{item} is unreachable in {item.location}')
def test_reversed_fill(self):
multi_world = generate_multi_world()
player1 = generate_player_data(multi_world, 1, 2, 2)