Added to Thriveopedia aproximate, page content and side panel search

[TomiYea]

Brief Description of What This PR Does

This PR updates the Thriveopedia search to allow approximate string matching in page names, and also search for the page’s content and side panel info.

Related Issues

Closes #4193

Progress Checklist

Note: before starting this checklist the PR should be marked as non-draft.

• PR author has checked that this PR works as intended and doesn't
  break existing features:
  https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
  (this is important as to not waste the time of Thrive team
  members reviewing this PR). This includes gameplay testing by the PR author.
• Initial code review passed (this and further items should not be checked by the PR author)
• Functionality is confirmed working by another person (see above checklist link)
• Final code review is passed and code conforms to the
  styleguide.

Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.

[adQuid]

Either a typo or not British English spelling in the name. A correct spelling of "additional" is here in the PR incidentally: https://github.com/Revolutionary-Games/Thrive/pull/7061/changes#diff-17a33871b0d2e73f93be118bc6bedbe0f40dfa3ea7e44e10c4f6e74b6bdbdd86R24

[hhyyrylainen]

I commented on all the typos I saw, apparently this had the same typo but I didn't notice in my initial review. So yeah fixing this as well would be good.

[revolutionary-bot]

The lead programmer for Thrive is currently on vacation until 2026-07-13. Until then other programmers will try to make pull request reviews, but please be patient if your PR is not getting reviewed.

PRs may be merged after multiple programmers have approved the changes (especially making sure to ensure style guide conformance and gameplay testing are good). If there are no active experienced programmers who can perform merges, PRs may need to wait until the lead programmer is back to be merged.

[hhyyrylainen]

Here's a quick re-review now that I'm back from my break. I didn't look at the core search algorithm again, but I think it was reasonable the last time I checked it?

[hhyyrylainen]

This needs some kind of caching as these kind of getters that build an absolute ton of temporary data are going to be really terrible if fetched more than once.

[hhyyrylainen]

Very good TODO observation here as well, this should be fixed before we consider merging this.

[hhyyrylainen]

Rather than using timer nodes, we usually just keep track of elapsed time in C# code using _Process delta parameter.

[hhyyrylainen]

Why is this needed now?

[TomasAntu]

SetParentPagesVisibility is currently only being called by DoBackgroundPageSearch which is background work
Directly doing parent.visible would throw errors

[hhyyrylainen]

Yeah, I think I saw that in my next comment (#7061 (comment)). So yeah that should be used instead (Invoke) and a comment here explaining why invoking on the main thread is needed would be very nice.

[hhyyrylainen]

Ah is this about being on the background thread?

If so we have a custom Invoke system that should be used instead for deferred actions that need to happen on the main thread.

[hhyyrylainen]

It looks like the node "searchThrottling" doesn't exist so shouldn't it be also removed from this change?

[hhyyrylainen]

I think I saw one bug remaining and a few general Thrive codestyle problems.

Can I ask honestly if you are using an LLM to make PRs? If so are my comments helping you improve your skills at all? If not I kind of unfortunately do not see much point in spending a bunch of time trying to guide you to making high quality Thrive contributions if you cannot really improve.

[hhyyrylainen]

Is it useful to list organelle names in the translation search? I'd say it is not as a big multicellular species can have 20 cells each with like 50 organelles, so this I think unnecessarily stresses the search system.

So basically any species would match any organelle search and I don't really see that being useful. Or what's your thought on that?

[TomasAntu]

I thought it would be useful for find species which could be toxic like whit oxytoxisome.
But it now seams that the search tags are providing more help

[hhyyrylainen]

I think that for truly finding species we would need a special search page for species, and it could have stuff like partial name, size (min, max), stage, has been engulfed by the player, has these organelles, doesn't have these organelles.

So I think it would make more sense to exclude search by organelle name in this PR.

[hhyyrylainen]

Suggested change

	if (trackSearchTimer && searchTimer > 0.1d)
	if (trackSearchTimer && searchTimer > 0.1)

What is that number format? I'm pretty sure it is not used anywhere else in Thrive.

[hhyyrylainen]

Suggested change

	searchTimer = 0.0d;
	searchTimer = 0;

[hhyyrylainen]

Suggested change

	private string currSearchText = string.Empty;
	private string currentSearchText = string.Empty;

I'm relatively sure that "curr" is not on our approved abbreviations list so it is not allowed to be used in the code.

[hhyyrylainen]

Suggested change

	// todo: maybe switch ToLower whit something else since it does return "a copy"
	// TODO: maybe switch ToLower whit something else since it does return "a copy"

[TomasAntu]

Can I ask honestly if you are using an LLM to make PRs?

That sucks to hear but no. This is my first contribution to any project and i'm new at this process of contributing.

[hhyyrylainen]

Oh no, sorry for assuming! We've been recently in the past few months hit with quite many AI PRs and they have been quite a pain to review. And unfortunately your PRs had the same feeling to them. Sorry if my words were too harsh.

This does change things, though. I will try to guide your PRs to the finish so that you can gain contributing experience. Hopefully you will stick around (for future PRs as well). It is extremely nice to see a new contributor who isn't just feeding us LLM output. And hopefully you can gain experience and level up your programming skills.

[hhyyrylainen]

I tested in the game and this feature seems to work, however I think maybe the search threshold needs tuning. I tried searching for "melanosome" and the "Museum" page appears and disappears multiple times while searching and "melanos" for some reason brings up the "Mechanics" page but I can't really find a reason for that appearing. So this feature is starting to be pretty good but needs tuning.

Besides that I found just a few quite minor code problems, however I think I saw one potential multithreading race condition which can cause a rare search not updating bug.

[hhyyrylainen]

Suggested change

	/// Has the input field changed while it still running a background search.
	/// Has the input field changed while it still running a background search?

Would this make the comment a bit clearer?

[hhyyrylainen]

Aren't these arrays? So it's a bit confusing I think to have "dictionary" in the name.

Also couldn't these be fields stored in this class? That way they can be reused and only need to be reallocated if the pages count changes.

[hhyyrylainen]

There's still some code issues (including a search flag race condition still) and I think the search fuzzy match is still not that well working.

If I search for "pri" then "Home" page is not found, but if I search for "prim" then it is and again if I search for "primum" then the "home" page disappears. So functionally I think it looks quite weird that pages disappear and reappear multiple times when you are typing a single word. I don't know enough about text search implementations to suggest a solution to this specific problem.

[hhyyrylainen]

I ended up reviewing the AI review notes and putting in my own thoughts and removing one thing that wasn't really right, so I don't want to just delete my notes so here's the full thing (note thought that I put in some caveats so if following these suggestions I think you should read them all before starting implementing any):

PR review findings

---

1. Background search reads and writes shared mutable fields without synchronization

DoBackgroundPageSearch() runs on a background task, but it reads/writes several fields also touched by the main Godot thread:

private bool requestingNewSearch;
private bool runningBackgroundSearch;
private bool isSearchDirty;
private string currentSearchText = string.Empty;
private int[] searchDistanceArray = Array.Empty<int>();
private bool[] searchDistancevisibility = Array.Empty<bool>();

Specific problems:

• OnSearchUpdated() writes currentSearchText, requestingNewSearch, isSearchDirty, and searchTimer on the main thread.
• DoBackgroundPageSearch() reads/writes isSearchDirty, requestingNewSearch, runningBackgroundSearch, currentSearchText, and the reusable arrays on a background thread.
• These are not volatile, not locked, and not protected by any synchronization primitive.
• Human NOTE: bool flags, I think, are supposed to be safe to read and set in C# so those specifically do not need synchronization but the arrays can need it
• This can cause stale reads, lost updates, or two tasks accidentally sharing state.

The array reuse is especially risky. If another search starts before the queued UI callback has consumed searchDistancevisibility, then the next background search can overwrite the same array before the first UI update runs.

Concrete bug

This part captures the field array indirectly:

Invoke.Instance.Queue(() =>
{
    iterator = 0;
    foreach (var page in allPages)
    {
        bool isVisible = searchDistancevisibility[iterator];
        // ...
    }
});

The callback does not capture a snapshot of results. It reads searchDistancevisibility later on the main thread. A newer background task may have already modified it, so an older queued UI update may apply newer/partial results.

Recommendation: use a local result array/list per search and capture that in the queued callback.
Human NOTE: probably better to use a search mutex to allow reusing the array to conserve on memory allocations

---

2. Out-of-order UI updates are possible

Even if the array issue is fixed, this design can still apply stale search results.

Sequence:

1. User types "a".
2. Search for "a" starts.
3. User types "ab".
4. Search for "a" queues a UI update.
5. Search for "ab" starts and queues another UI update.
6. Depending on queue timing, stale "a" results may be applied after "ab" results.

There should be a monotonically increasing search version / generation id. The UI callback should only apply results if it is still the latest search.

Example concept:

private int searchGeneration;

Then increment it when the query changes, pass the captured generation into the background search, and ignore queued results if the generation no longer matches.

---

3. runningBackgroundSearch can get stuck true

At the end of DoBackgroundPageSearch():

if (requestingNewSearch)
{
    TaskExecutor.Instance.AddTask(new Task(() => DoBackgroundPageSearch(currentSearchText)));
    requestingNewSearch = false;
}
else
{
    runningBackgroundSearch = false;
}

If requestingNewSearch is true, a new search is queued, but runningBackgroundSearch remains true. That appears intentional.

However, because requestingNewSearch is unsynchronized, this can lose updates. For example:

1. Background thread sees requestingNewSearch == true.
2. Main thread receives another search update and sets requestingNewSearch = true.
3. Background thread queues the new task and then sets requestingNewSearch = false.
4. The main thread’s newer request can be overwritten/lost.

Then the last typed search may never run.

Human NOTE: recommendation to set requestingNewSearch to false before calling AddTask.

---

4. isSearchDirty is cleared from the background thread

DoBackgroundPageSearch() starts with:

isSearchDirty = false;

The background search should not clear the “dirty” state for input changes that may have happened after that search was scheduled.

BeginBackgroundSearch() already clears isSearchDirty before scheduling the task:

isSearchDirty = false;

So the assignment inside DoBackgroundPageSearch() should likely be removed.

---

5. Godot / UI / localization access from background thread may be unsafe

The background task calls:

page.Key.TranslatedPageName
page.Key.TranslatedPageBody
page.Key.TranslatedAdditionalSearchContent
Localization.Translate(...)
PageContent.Sections
PageContent.InfoboxData

Depending on implementation details, this may touch Godot objects, UI nodes, localized resources, or data structures not intended to be accessed off the main thread.

This is especially concerning for ThriveopediaWikiPage.TranslatedPageBody, which lazily builds and caches translated text from page content:

if (cacheTranslatedPageBody == null)
{
    StringBuilder builder = new StringBuilder();
    foreach (var item in PageContent.Sections)
    {
        builder.AppendLine(Localization.Translate(item.SectionBody));
    }

    cacheTranslatedPageBody = builder.ToString();
}

That cache can also be reset by OnTranslationsChanged() on the main thread while a background search is reading/writing it. This is another race.

Recommendation: snapshot all searchable strings on the main thread, then do only pure string matching on the background thread.

Human NOTE: this seems a bit excessive, I think it would be good enough to once call all Translated* methods on the main thread before the first search after translations change. This caches all the strings in C# and no longer calls Godot, and the user should take a while after searching to be able to navigate to the options and swapping language so the race condition here is not a threat.

---

6. Empty search probably returns incorrect results

With newText == "", this code does:

var newTextLowercase = newText.ToLower(CultureInfo.CurrentCulture);

Then every page name/body/additional content contains "", so all non-stage pages become visible and stage pages cause the stage dropdown to show.

This may or may not match previous behavior. The old code also used:

TranslatedPageName.ToLower(...).Contains(newTextLowercase)

so it likely had the same behavior for empty search. But with the body/additional search and fuzzy logic, this behavior should be explicitly checked. Usually empty search should reset the tree to the normal Thriveopedia state rather than showing all pages.

---

7. searchDistanceArray.Min() can throw if there are no pages

var costThreshold = searchDistanceArray.Min();

If allPages.Count == 0, this throws InvalidOperationException.

Maybe allPages is guaranteed to be non-empty in practice, but this is still fragile. Also note that if searchDistanceArray.Length is larger than allPages.Count, Min() checks unused entries too.

This is a real bug because the array is reusable and can be larger than the current number of pages:

if (allPages.Count > searchDistanceArray.Length)
{
    searchDistanceArray = new int[allPages.Count];
}

If the number of pages ever decreases, or if the array was oversized for any reason, searchDistanceArray.Min() includes stale/default elements outside the range populated by the current loop.

Use only the populated count:

var costThreshold = searchDistanceArray.Take(allPages.Count).Min();

Human NOTE: we can probably put a general check in the Thriveopedia to throw if pages is empty and ignore that, but the page count decreasing is an actual problem that can trigger.

---

8. Fuzzy search threshold is probably wrong

This line makes the threshold equal to the single best page-name distance:

var costThreshold = searchDistanceArray.Min();

Then this condition includes only pages whose title distance equals the current global minimum:

searchDistanceArray[iterator] <= costThreshold

Because costThreshold is the minimum, this only includes best-match pages by distance. That is not really “a threshold for similar results”.

It may also behave badly with an empty/short query. For example, searching "a" will probably make the “closest” title whichever page has shortest length / nearest edit distance, not necessarily a useful result.

A threshold usually needs to be based on query length and/or page-name length, for example:

distance <= Math.Max(1, newTextLowercase.Length / 3)

or normalized distance:

distance <= maxLength * 0.3

---

9. The distance is computed against full page names only

StringUtils.DoStringCostBetween(pagename, newTextLowercase);

For multi-word page titles, full-string Levenshtein distance can be poor. Searching for one word in a long title will have a high distance even if it matches a word closely. The later Contains() helps exact substrings, but not typo substrings.

For example, if title is "Evolutionary Tree" and user searches "evolutonary", full-title distance includes the " Tree" suffix as extra deletions. A word/token-based fuzzy comparison would produce better results.

---

10. Possible closure issue with iterator

This part mutates a local variable after it has been captured by a lambda:

Invoke.Instance.Queue(() =>
{
    iterator = 0;
    foreach (var page in allPages)
    {
        bool isVisible = searchDistancevisibility[iterator];
        // ...
        ++iterator;
    }
});

Then after queueing the lambda, the method continues and may return or start another search. Since iterator is captured, not copied, this is brittle. It’s probably not a correctness issue as written because the lambda resets it to 0, but it is an unnecessary closure over mutable state. Use a local variable inside the lambda instead:

Invoke.Instance.Queue(() =>
{
    var index = 0;
    foreach (var page in allPages)
    {
        bool isVisible = visibility[index];
        // ...
        ++index;
    }
});

---

11. Typo / naming issues

Minor, but worth fixing:

private bool[] searchDistancevisibility = Array.Empty<bool>();

Should be:

private bool[] searchDistanceVisibility = Array.Empty<bool>();

Comments also have typos:

• “whit” → “with”
• “it still running” → “it is still running”

---

12. Levenshtein distance function

The diff does not include StringUtils.DoStringCostBetween, so I can’t verify the implementation directly from the provided diff.

But based on how it is called, check the function for these requirements:

• distance("", "") == 0
• distance("a", "") == 1
• distance("", "abc") == 3
• distance("kitten", "sitting") == 3
• distance("flaw", "lawn") == 2
• distance("gumbo", "gambol") == 2
• It should be symmetric:
  • distance(a, b) == distance(b, a)
• It should not read outside array bounds for empty strings.
• If it uses a one-row DP array, it must preserve the previous diagonal value correctly.

A correct one-row Levenshtein implementation looks like this:

public static int LevenshteinDistance(string first, string second)
{
    if (first.Length == 0)
        return second.Length;

    if (second.Length == 0)
        return first.Length;

    var costs = new int[second.Length + 1];

    for (int j = 0; j <= second.Length; ++j)
        costs[j] = j;

    for (int i = 1; i <= first.Length; ++i)
    {
        var previousDiagonal = costs[0];
        costs[0] = i;

        for (int j = 1; j <= second.Length; ++j)
        {
            var oldCost = costs[j];

            var substitutionCost = first[i - 1] == second[j - 1] ? 0 : 1;

            costs[j] = Math.Min(
                Math.Min(costs[j] + 1, costs[j - 1] + 1),
                previousDiagonal + substitutionCost);

            previousDiagonal = oldCost;
        }
    }

    return costs[second.Length];
}

---

Suggested direction

The safest design would be:

1. On main thread, debounce input.
2. On main thread, create an immutable snapshot:
   • page reference / tree item reference
   • translated name
   • translated body
   • translated extra search content
3. Start background task using only that snapshot.
4. Produce a local bool[] or result list.
5. Queue UI update with captured local results.
6. Apply only if a captured search generation is still current.

Human NOTE:
It's probably better to have one-time allocated data for all of the above and a SemaphoreSlim which controls when writing to the data is allowed. So check if can take the lock on the main thread: if not wait until the next _Process call to check again, if can take the lock fill in the data on the main thread to all the stuff and start a task, the task then runs and completes and signals to the main thread it is ready, the main thread then applies the results and only then releases the semaphore which allows a new search to fill its data and start if things have been marked dirty by the search changing.

Pseudo-shape:

private int searchGeneration;

private void OnSearchUpdated(string newText)
{
    currentSearchText = newText;
    isSearchDirty = true;
    searchTimer = 0;
    ++searchGeneration;
}

private void BeginBackgroundSearch()
{
    if (runningBackgroundSearch)
        return;

    isSearchDirty = false;
    runningBackgroundSearch = true;

    var generation = searchGeneration;
    var text = currentSearchText;

    var snapshot = allPages
        .Select(page => new SearchablePage(
            page.Value,
            page.Key is ThriveopediaStagePage,
            page.Key.TranslatedPageName,
            page.Key.TranslatedPageBody,
            page.Key.TranslatedAdditionalSearchContent))
        .ToArray();

    TaskExecutor.Instance.AddTask(new Task(() => DoBackgroundPageSearch(generation, text, snapshot)));
}

The important part is that the worker task should not touch Godot objects (human note: this is not fully true), mutable shared arrays, or localization/page properties (human note: and this is definitely not true as the localization system uses cached strings after first access). It should only process immutable strings and then queue a guarded UI update.

[TomasAntu]

The current commit is a work in progress from the review.
I'm doing this because i will not be available until 3 of august and so that @TomiYea (if able) can work on it.
Even if I'm not going to be able to work on it now , i do have a question regarding the 2º point from the review since i am having an hard time trying to understand what and why its recommending.

[hhyyrylainen]

I think there is still a possibility of the data getting out of sync because the invoke queue may happen after this code, and it thus would allow the arrays lock to be grabbed before the invoke runs. I think.

So my suggestion is to move this setting false inside the invoke, that ensures that a new search can only begin after the results of the previous one are applied. Also the main thread potentially gets blocked for the entire duration of the search, if a new search operation was to start before the invoke runs. So for that reason as well I think this flag reset should be inside the invoke so that nothing can grab the array lock and stall the main thread.

[hhyyrylainen]

I think this needs a safety check, because technically it looks to be possible for a new page to be added before the results of a search are applied, so this array may not match the size of the pages count.

[hhyyrylainen]

Suggested change

	// TODO: maybe switch ToLower whit something else since it does return "a copy"
	// TODO: maybe switch ToLower with something else since it does return "a copy"

[hhyyrylainen]

I did a new look and I think the safety is much better now. Regarding the number 2 point in the review and what I just commented on: it looks to be possible to get data out of order: because the results are invoke'd it means that they are not immediately applied, so a new search has a chance to start before the results of the old one are applied, and now with the lock that will stall the main thread and it will then apply the results of the new search twice in a row. I think that bug can be removed by ensuring that only after applying the results the flag to allow a new search to begin is reset. This way a partial search cannot be interfered with by a new one starting. That's mostly also the gist of the older review point 2 (interleaving of results from different searches).

[TomasAntu]

While working on the latest commit i notice something that might by a huge problem.
Searching for "nucleus" results in a unhelpful results since the info box of organelles contains "Requires Nucleus" and organelles that require a nucleus have it in the requirements part of their pages body text making it show every single organelle.
The idea i had is maybe sorting the tree by relevance (title then extra search then body text).
I'm not sure how much of an issue this is to need it, since it only affects the nucleus but it is an important progression step.

[hhyyrylainen]

The idea i had is maybe sorting the tree by relevance (title then extra search then body text).
I'm not sure how much of an issue this is to need it, since it only affects the nucleus but it is an important progression step.

I think sorting by relevancy would be pretty good. Or alternative and idea I just had is that if a page title matches (or there's good results already), then the page body search would be skipped as they are likely less relevant results. I think that would help in finding the nucleus.

Sorting the tree items might be somewhat complicated as it could have quite unexpected bugs show up.

[hhyyrylainen]

So is this now functionally ready?