BestLeastRecently.* Saliency Strategy documentation misleading

[kinggra1]

Saliency Strategy Documentation seems to indicate that the possible options will be sorted by Score and then by Views for each of the "BestLeastRecnetly.*" strategies, but the code seems to do the opposite (sorts by views, then sorts by complexity score)

Random Best Least Recently Viewed: The items that have not failed any conditions are sorted by score, and then by how many times they have been selected by this strategy. If there is more than one best item remaining, a random one of these is selected.

https://github.com/YarnSpinnerTool/YarnSpinner/blob/f7c45109d53ffb78124575e57688421b5d0fa7f8/YarnSpinner/Saliency/RandomBestLeastRecentlyViewedSaliencyStrategy.cs#L90C1-L101C25

Not sure if this is a documentation clarity issue, but I expected the opposite behavior (among all options with the highest complexity, select the least recently viewed one) not what I've tested (among all options that have been viewed the last # of times, select the one with highest complexity)

Not blocking, just writing my own custom strategy now that I know, but wanted to check for future reader clarity :)

Thanks!

[TheMartianLife]

Great point. I also thought it was the way it says in the documentation. If we want to keep it View>Complexity, then there will be other docs which need to be clarified as well, including the recent blog post about Saliency traps.

[kinggra1]

My 2 cents would be to favor Complexity>View (i.e. bugfix instead of documentation change). It felt unusual to me in terms of saliency to wind up cycling through all valid dialogue (including the when: always option) one full time before I can get back to an important high-complexity salient storylet.

That being said, I totally understand how this could be workflow breaking depending on how others have been using it. And the naming convention seems self-consistent as is: RandomBestLeastRecentlyViewed is Random < Best < LeastRecentlyViewed

[TheMartianLife]

I would vote for the same. This is probably a @McJones decision.

[desplesda]

We've had some conversations internally about how we want this to work.

• If you group by view count and then by score, you end up seeing every item, with the highest score items first.
• If you group by score and then by view count, you end up seeing only the items that have the highest score.

(Just to be sure, we ran some tests on it to confirm that this behaviour is actually happening, and it is.)

Our thinking is that, unless content should not run at all (i.e. it's failed a condition), it should be eligible for being shown. Higher-scoring content should definitely be shown first, but if the game repeatedly needs to show content and it's already shown the best stuff, it's better to show new (albeit less-relevant) stuff than repeat the best stuff.

To that end, we'll:

1. Leave the behaviour as-is;
2. Update the documentation to reflect its actual behaviour.

Thanks for the bug report!

[TheMartianLife]

Un-closing this because this actually then has to change in a bunch of places not just the docs page for Saliency Strategies. So I'll mark this Followup and compile a list of places which need to change that we can check off.

Off the top of my head, we need to at least check (if not change):

• Saliency Strategy docs
• Saliency sample guides
• Saliency sample in-game dialogue/explanations
• Doc comments in the code
• Tim's blog post about saliency and state mutation
• Tim's video-in-preparation about saliency strategies