Add more vault search options#1154
Merged
Gatsik merged 5 commits intoFAForever:developfrom Feb 14, 2026
Merged
Conversation
game process is not actually starting here also, there are messages about starting ICE adapter immediately after it which made this message to be seen only for a fraction of a second
📝 WalkthroughWalkthroughAdds ETFREEMAN as a third unit-db option, new search parameter UIs for map and mod vaults, centralizes vault filter construction, updates related CSS/theme and UI files, introduces MapSize.from_side_km, and minor client UI/thumbnail tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Vault UI (map/mod)
participant Vault as Vault controller
participant API as ApiClient
participant Browser as External Browser/UnitDB
User->>UI: enter filters / click Search
UI->>Vault: construct_search_filters()
Vault->>API: query with QueryOptions
API-->>Vault: results
Vault-->>UI: render results
User->>UI: click ETFREEMAN button
UI->>Browser: open_db_url(index) (UNITDB_ETFREEMAN_URL)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@requirements.txt`:
- Line 1: The requirements currently include pinned packages but must not
include pytest==9.0.2 due to CVE-2025-71176; update requirements.txt to either
pin pytest to a safe version (e.g., pytest==8.4.2), remove or delay adding
pytest 9.0.2, or document using a secure basetemp invocation (pytest
--basetemp=/secure/path) as a temporary mitigation; ensure the change is applied
where pytest is declared (requirements.txt) and mention the chosen mitigation in
the PR so reviewers know which option was taken.
- Line 13: requirements.txt pins pytest==9.0.2 which requires Python ≥3.10 and
introduces breaking changes; update CI/test matrix to use Python 3.10+ (or pin
pytest lower), search the codebase for deprecated pytest patterns (e.g., calls
to pytest.importorskip() without exc_type and any usages that may trigger
PytestRemovedIn9Warning), bump test-related plugins to compatible versions
(pytest-cov ≥7.0.0, pytest-mock ≥3.15, pytest-qt ≥4.5.0) in requirements/dev
files, and run the full test suite with pytest==9.0.2 to identify and fix any
failing tests or deprecation-as-error issues before merging.
In `@src/vaults/mapvault/mapvault.py`:
- Around line 64-86: construct_search_filters interpolates raw user input into
filter expressions (displayName and author.login) which can break the filter
syntax; sanitize or escape special characters before building the filter
strings. In construct_search_filters (and the analogous method in modvault.py)
read the text from searchParams.searchInput and authorInput, normalize it, then
escape or remove characters that can break the query (at minimum: single quote
', semicolon ;, equals signs ==, and wildcard *), or better yet implement a
quoting/escaping helper (e.g., escape_filter_value(value: str) used by
construct_search_filters) that replaces/escapes these chars and ensures the
final value is wrapped/quoted correctly before embedding into
f"displayName=='*{escaped}*'" and f"author.login=='*{escaped}*'". Ensure you
update both displayName and author.login usages to call this helper and add unit
tests for inputs containing ', ;, =, * to verify correct behavior.
🧹 Nitpick comments (2)
src/vaults/vault.py (2)
41-41: Empty-string default for_search_params_uicould silently break subclasses.If a new vault subclass forgets to assign
_search_params_uibefore callingsuper().setup(),util.THEME.loadUi("")will fail at runtime with an unhelpful error. Consider raising explicitly if the value is still empty atsetup()time.💡 Proposed defensive check in setup()
def setup(self) -> None: self.setupUi(self) + if not self._search_params_ui: + raise ValueError( + f"{type(self).__name__} must set _search_params_ui before calling setup()", + ) self.searchParams = util.THEME.loadUi(self._search_params_ui)
257-258: Consider usingabc.abstractmethodinstead of raisingNotImplementedError.Using
@abc.abstractmethodwould catch missing implementations at class instantiation time rather than at call time, providing earlier and clearer error feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Chores