Add priority field to zones, use in loot sorting.

[Stu33]

Summary

Features "Add priority field to zones, use in loot sorting."

Purpose of change

Sometimes there are multiple zones that might be valid to place an item into when sorting the loot or placing NPC items. The game already has a priority ordering by zone type (custom filter > special cases > specific category > default) but it doesn't always do what the player would like, particularly if there are multiple custom loot zones with filters that match the item. Adding a priority field to zones that the player can edit allows them to get items to go where they want them.

In the future a priority field would be useful for other zone-based activities as well, for example specifying the order in which an NPC should try to complete construction blueprints.

Describe the solution

Add a priority attribute to zones, defaulting to 0. Display it in the zone manager UI and allow it to be set there. The priority may be set to any positive or negative value within the range of an integer.

Use this priority field to select which zone loot is moved to if more than one zone is a suitable target for a loot sorting activity. Zones with higher priority are considered first, even if they would have been disregarded in the previous system (e.g. if you have a priority 1 default loot zone then even weapons will go there rather than to a weapon loot zone with priority 0).

Because this is implemented by changing the zone_manager::get_near_zone_type_for_item method, it also affects placement of NPC items in shop restocking, trade and drops as the result of NPC missions, but unless the player or another PR deliberately changes zone priorities from the default this will have no visible effect.

Describe alternatives you've considered

None. In previous games I have either under-used zones or carefully set up custom filter zones to avoid having more than one possible destination for an item in loot sorting.

Testing

• Loaded and saved a pre-existing game. Observed that "priority":0 is stored for each zone in zones.json. Edited the priority of some zones, saved game and observed correct priority values stored in zones.json. Played for a while and noticed nothing wrong.
• Tested the specific reproduction case noted below.
• Tested having an NPC sort loot for multiple other zone setups.
• Checked that the existing unit tests in clzones_test.cpp and zones_custom_test.cpp continue to pass.
• Added new tests - sets named zone_containment and zone_sorting_priority in clzones_test.cpp.

One quick and easy case to demonstrate this, using this zipped save:
repro.zip

• Existing behaviour:
  1. Load the saved game from the attached zip in the latest experimental build without this PR (ignore the unrelated warning message - it's a save from a slightly older build of the game).
  2. Ask the NPC standing next to you to sort the loot, then wait around until they're done.
  3. See that all of the emergency jackets and emergency blankets have been moved to the custom loot zone with filter "emergency", and nothing has gone in to the zone with filter "emergency blanket". Nothing has gone to the Default zone.
• To test that priorities work:
  1. Load the save from the attached zip with this PR's changes applied.
  2. Open the zone manager, select the custom loot zone with filter "emergency blanket" and change its priority to 1 (or higher) using the new UI.
  3. Ask the NPC standing next to you to sort the loot, then wait around until they're done.
  4. See that the emergency blankets have moved to the "emergency blanket" zone, and emergency jackets have still gone to the "emergency" zone.
• To test that negative priorities are treated as lower priority than the default:
  1. Load the save from the attached zip with this PR's changes applied.
  2. Select the custom loot zone with filter "emergency" and change its priority to -1 (or lower).
  3. Ask the NPC standing next to you to sort the loot, then wait around until they're done.
  4. See that the emergency blankets have moved to the "emergency blanket" zone, and emergency jackets have now gone to the Default zone.

Additional context

Examples of the changed zone manager:

New option to set priority in the zone editing menu:

Popup to set the priority:

[andrei8l]

Why do you need a separate field for priority when you already have order in the list? Just make zones closer to the top take precedence, which IIRC is already the case within a type.

[Stu33]

Why do you need a separate field for priority when you already have order in the list? Just make zones closer to the top take precedence, which IIRC is already the case within a type.

To me the priority and positioning in the display are different concepts. I like to order the zones for display within the zone manager by world position so that I can find them more easily. I often have a daft number of zones.

But I'd be happy to drop the new field and just use order in the list instead if that's the consensus.

[Termineitor244]

I'm in favor of separating zone order and priority, when I make the zones for my base I make them and organize them in order with their distance from one another, so that I can easily locate them.

[o175]

@Stu33 What do you think about viewing \ or even editing priorities from the main zone manager table?

[GuardianDll]

Please resolve clang tidy errors

[Stu33]

I like the idea of displaying the priority on the main zone list, but it's already short on space for displaying zone names. Perhaps I could investigate that in a new PR if this one and #79347 are accepted?

…

On Mon, 27 Jan 2025, 05:30 o175, ***@***.***> wrote: @Stu33 <https://github.com/Stu33> What do you think about viewing \ or even editing priorities from the main zone manager table? — Reply to this email directly, view it on GitHub <#79353 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXRMU7454FMSMU3K7YKHCAD2MW76BAVCNFSM6AAAAABV4RYDFKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJUHA3TQNBTGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[GuardianDll]

I personally think you may be better with moving zone stuff to imgui, it sound like having proper mouse and better UI would benefit it, and make your further work much easier

[GuardianDll]

Was the issues raised in #35782 resolved?

[Stu33]

Sorry, I missed #35782 when considering this. Thanks for pointing it out. My changes have exactly the same problem in terms of turning this into an exhaustive search of the zones, although most should be rejected pretty quickly if they don't match. I've not noticed that causing a problem in my current game, but I can see the point Kevin made back then about performance issues in the future or for situations I've not seen. I'll look into what I can do to fix that.

[Stu33]

RE the UI, I had hoped to just learn one are of code at a time, but I agree moving to imgui is a good idea. I'm happy to do it, but perhaps that would belong in a new PR rather than making this one bigger?

[GuardianDll]

Sure, i just point out that it would be easier for you to start with imgui and then expand it than the other way around; no issues finishing this and another PRs of yours before doing UI

[Stu33]

(TLDR; skip to the numbered list below and express a preference please?)

I can see a few ways to proceed. I wonder what anyone thinks is best? Given Kevin's comments on #35782, perhaps Kevin or someone else more familiar than me with plans to use zones in faction camps and other NPC activities could comment?

This change affects only the users of the zone_manager::get_near_zone_type_for_item() method, but like I said in the description that does include loot placement from NPC missions and trader goods placement as well as loot sorting activities.

The implementation before this PR did a search through LOOT_CUSTOM zones, then all LOOT_ITEM_GROUP zones, returning as soon as it found a matching zone within range. If none of those matched it would determine which zone types could match the item and search for near points in the caches for those zone types and matching faction in a specific priority order. Note at this point it just finds the best zone type ID, not the best point; the loot sorting activity then calls zone_manager::get_near to get the std::unordered_set of all points of a matching zone type within range, so there is no kind of priority applied between zones of the same type unless by accident of the implementation of std::unordered_set on a given platform. It's also going to do the exhaustive search through custom and item group zones even if the correct match ends up being a firewood zone, weapon zone or whatever.

My initial implementation instead did an exhaustive search through all zones, quickly skipping any that don't have a matching faction, are disabled or are lower priority than the best seen so far. Then it checked whether the zone's bounding rectangle was within range and whether the zone was a suitable destination. It does need to iterate through all zones rather than returning as soon as a match is found. I've done my best to keep that quick, but if there are large numbers of zones that are not custom or item group ones then it probably will be slower than the pre-existing code. Having determined the best zone ID and priority, it then calls an overload of zone_manager::get_near to make another search through all the zones and calculate covered point sets; I think this second call could be mostly optimised away by making the first search optionally collect covered points as it searches, or it could make some use of the point caches, but I didn't do that yet.

Possible solutions to allow this PR to proceed without hurting other uses of zones:

1. Apply priority only to custom and item group zones, which the pre-existing code iterates exhaustively anyway.
2. Restrict priority to a small range of values, and use those as part of the zone cache key alongside the currently used faction and zone ID.
3. Separately cache priority ordered lists of the zones with priority greater and less than the default, and only exhaustively search the higher priority zones before using the pre-existing method for the default priority zones (then search less than default priority only if no default priority zone matches).
4. Any other suggestions?

Presumably the non-default priority zones will just be user-added zones, so there won't be many of them. This means most of the extra caches in solution 2 and the priority ordered lists in 3 will be small. Option 1 is a much smaller change to the pre-existing code and I think is still better than having no priority control.

If nobody else has an opinion I'll go ahead and implement option 3 from that list. I think this will have minimal impact on existing uses that stick to default zone priority while allowing maximum flexibility for zones the player configures. But please tell me if you think differently or if this PR won't be accepted regardless of any of those choices.