Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Advanced Search updates #6131

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

SJuliez
Copy link
Member

@SJuliez SJuliez commented Oct 21, 2024

This PR

  • Reworks the weapons&equipment tab so the filter summary does not vanish at the bottom of the window (not part of the scroll pane); makes the weapon and equipment tables less tall
  • Adds a text filter to the weapons&equipment tab that filters the tables according to the input
  • reworks all scrollpanels to be less obnoxious
  • Uses FlatLaf's own tristate checkbox in the unit type tab
  • On the code side, takes apart TWAdvancedSearchPanel (and removes an old copy of it that wasnt used anymore)
  • moves all the search stuff into its own package

image

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link

codecov bot commented Oct 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 29.19%. Comparing base (c6d30f7) to head (3b8e964).
Report is 25 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6131      +/-   ##
============================================
+ Coverage     29.07%   29.19%   +0.12%     
+ Complexity    13972    13970       -2     
============================================
  Files          2601     2620      +19     
  Lines        267447   266334    -1113     
  Branches      47758    47567     -191     
============================================
- Hits          77767    77764       -3     
+ Misses       185782   184674    -1108     
+ Partials       3898     3896       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


// Parsing Parenthesis
if (filterTok instanceof TWAdvancedSearchPanel.ParensFT) {
if (((TWAdvancedSearchPanel.ParensFT) filterTok).parens.equals("(")) {
if (filterTok instanceof ParensFT) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the later ones can be simplified with the instanceof cast from java 17

@HammerGS
Copy link
Member

Have some ideas and suggestions.

  • Some equipment with variable costs shows as -2147483648 would it be possible to just show variable. (i.e AES)

  • Some weapons have negative damage (i.e. Mech Mortars) since the damage is ammo based would it be possible to show variable as well.

  • One mentioned on Discord, would be it possible to add the ability to search for Movement to the Total Warfare search versus blending with Alpha Strike.

Noticed this.
image

Not sure if it's us not tagging the naval weapons or a filtering issue. As the NAC's seem to filter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants