Change Important Check Hints for Single-Location Areas

[jdunn596]

Closes #2501 if accepted

This PR does a couple things. First, a check has been added to ensure that areas that have all locations already hinted cannot be selected for an important check hint. Once the hinted location has been selected, it is then added to the checked dict directly (in addition to HintArea being added as well) to ensure that other hint types cannot hint that location.

Testing

• Added 'Wasteland Chest' as an always hint on S9 settings. Checked to make sure that HintArea.HAUNTED_WASTELAND was not added to top_level_locations. Turned Overworld Skulls on and re-ran test to make sure HintArea.HAUNTED_WASTELAND was now included.
• Forced the hint area to be HintArea.HAUNTED_WASTELAND with normal S9 settings without Wasteland being hinted already. Checked to make sure that 'Wasteland Chest' was marked in checked dict.

[shirosoluna]

Please also make sure this works in No Logic. It broke on no logic seeds previously and has since been fixed so this behavior can break that again.

[jdunn596]

With minimal testing, no logic seeds seem to generate without issue

[shirosoluna]

With minimal testing, no logic seeds seem to generate without issue

specifically just use the important check hint distro. This generates one for every region.

[jdunn596]

Okay they generate just fine but there is a slowdown in generation time due to shuffled_locations_in_region = list(filter(lambda loc: HintArea.at(loc) == hint_area and not loc.locked, world.get_filled_locations()))

converting the filter object to a list seems pretty taxing so I need to figure that out

Other than that, nothing actually breaks for both glitchless and nologic

[shirosoluna]

Okay they generate just fine but there is a slowdown in generation time due to shuffled_locations_in_region = list(filter(lambda loc: HintArea.at(loc) == hint_area and not loc.locked, world.get_filled_locations()))

converting the filter object to a list seems pretty taxing so I need to figure that out

Other than that, nothing actually breaks for both glitchless and nologic

might all well check the full trifecta with advanced lol

a slowdown sounds really weird. It never did that before. Is it something to do with not being able to determine location locked items?

[flagrama]

Why are you converting it to a list? in should work with any iterable, including the one returned from filter.

[jdunn596]

Why are you converting it to a list? in should work with any iterable, including the one returned from filter.

It's a check to see how many shuffled areas are in the region of the location currently being checked so my thought was to check the length of that list to see if that is the only shuffled check in the region. If so, we mark that location specifically as hinted in addition to the hint area so that specific location cannot be selected for another hint type.

The problem is that you can't call len() on the filter object so it had to be converted.

Upon further testing though, that doesn't seem to be what's slowing it down. Specifically line 1186 is slowing it down whether it's a list or filter object. Line 1164 doesn't seem to affect it. I'm gonna convert this to draft for now because I don't fully understand what's causing the slowdown

[flagrama]

Ah, I did miss that you were using len on it. Odd that both lines look like they should be doing the same thing but take a different amount of time. I'm assuming you've actually used a profiler to determine the second one is taking longer? Or are you just going by how long each takes in the debugger?

My first recommendation is to not use get_filled_locations() in the loops as it is also a filter. I'd create a list from it outside of each loop that you then filter on without invoking the function's inner filter on each iteration of the loop. Since filled locations won't (can't) be added during the loop this will just reduce unnecessary processing. Maybe that will help.

Also if you are relying on the debugger to assess "speed of execution" I recommend figuring out how to set up a profiler for Python and use that to determine where the code hotspots are. Or at least rely on time logging in a non-debug environment. I don't really know how to set up a profiler for Python, my IDE just does all the backend work to set one up for me I'm pretty sure.

[jdunn596]

Ah, I did miss that you were using len on it. Odd that both lines look like they should be doing the same thing but take a different amount of time. I'm assuming you've actually used a profiler to determine the second one is taking longer? Or are you just going by how long each takes in the debugger?

My first recommendation is to not use get_filled_locations() in the loops as it is also a filter. I'd create a list from it outside of each loop that you then filter on without invoking the function's inner filter on each iteration of the loop. Since filled locations won't (can't) be added during the loop this will just reduce unnecessary processing. Maybe that will help.

Also if you are relying on the debugger to assess "speed of execution" I recommend figuring out how to set up a profiler for Python and use that to determine where the code hotspots are. Or at least rely on time logging in a non-debug environment. I don't really know how to set up a profiler for Python, my IDE just does all the backend work to set one up for me I'm pretty sure.

Thanks for the suggestions. I set up cProfile (standard library) and snakeviz (prof visualizer). I ended up moving the 2nd shuffle_locations_in_region inside the if HintArea.at(location) == hint_area condition under line 1187 and it mitigates the issue there. But now apparently the first call is in fact slowing things down (it didn't seem that way in initial testing) which makes sense so I'm gonna keep messing with that. I think moving it inside of:

if (
     hint_area not in top_level_locations
     and hint_area not in checked
     and hint_area != HintArea.ROOT
     and hint_area.dungeon_name not in empty_dungeons # prevent pre-completed dungeons from being hinted
     and not location.locked # prevent areas with unshuffled checks from being hinted
):

will help if that condition is met

EDIT: Yeah confirmed this fixes the issue

[flagrama]

You might even be able to get away with just the first assignment of filled_locations and use that in place of all of these world.get_filled_locations() calls since nothing here looks like it should be modifying what gets returned by that function to me. But I do only have the context of this small section of one file here.

[jdunn596]

You might even be able to get away with just the first assignment of filled_locations and use that in place of all of these world.get_filled_locations() calls since nothing here looks like it should be modifying what gets returned by that function to me. But I do only have the context of this small section of one file here.

Went with what you said yeah. It's definitely better. Had to write locations = [location for location in world.get_filled_locations()] because the filter is an iterator