-
Notifications
You must be signed in to change notification settings - Fork 292
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
Move Field of Fire Sprites out of BoardView #5466
Conversation
… into clientgui-boardviews # Conflicts: # megamek/src/megamek/client/ui/swing/AbstractClientGUI.java
…into clientgui-boardviews # Conflicts: # megamek/src/megamek/client/ui/swing/ClientGUI.java
… clientgui-boardviews
…ts' into clientgui-boardviews # Conflicts: # megamek/src/megamek/common/strategicBattleSystems/SBFGame.java # megamek/src/megamek/server/SBFGameManager.java
} | ||
|
||
|
||
public Map<Integer, List<Report>> createFilteredReport(Player recipient) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preliminary code (recipient will be needed at some point)
private final GameListener gameListener = new SBFClientGUIGameListener(this); | ||
private final CommonMenuBar menuBar = CommonMenuBar.getMenuBarForGame(); | ||
|
||
public SBFClientGUI(SBFClient client, MegaMekController c) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preliminary code
private static final String FILENAME_ICON_32X32 = "megamek-icon-32x32.png"; | ||
private static final String FILENAME_ICON_48X48 = "megamek-icon-48x48.png"; | ||
private static final String FILENAME_ICON_256X256 = "megamek-icon-256x256.png"; | ||
public class ClientGUI extends AbstractClientGUI implements BoardViewListener, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so much cleaner - nice work.
import java.util.Set; | ||
|
||
/** | ||
* This BoardViewSpriteHandler handles the sprites for the firing arcs (field of fire) that can be shown for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for putting "firing arc" in the comments; "field of fire" is not intuitive to me and I'm constantly struggling to find this kind of code because of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was having trouble finding the GUIP setting myself :D And I added that whole feature originally (was merged at the time by arlith as I had no clue about PRs. Ten years ago)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure it actually predated GitHub when we where on Sourceforge and you had to do patches via Subversion. You're patch generated a lot of buzz in the dev team at that time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few questions and a couple of lines that were commented out.
It would also be good to attend to the codeQL issues.
Would it be possible to add some more info about these changes to the commit summary?
Specifically, the addition of some SBF and Alpha Strike functionality goes well beyond moving Field of Fire sprites, and should probably be at least mentioned.
Otherwise, looks good!
This is why I wrote in the first post that this would best be reviewed after merging the previous PRs because this builds on them. After merging previous PRs and updating this one it will become much smaller :) |
It is not my intention to create useless effort. I should have made this one a draft. Mea culpa |
# Conflicts: # megamek/src/megamek/client/ui/swing/ClientGUI.java # megamek/src/megamek/common/strategicBattleSystems/SBFGame.java
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5466 +/- ##
============================================
- Coverage 29.27% 29.27% -0.01%
- Complexity 13650 13686 +36
============================================
Files 2434 2440 +6
Lines 261580 261813 +233
Branches 46676 46684 +8
============================================
+ Hits 76588 76646 +58
- Misses 181167 181350 +183
+ Partials 3825 3817 -8 ☔ View full report in Codecov by Sentry. |
This PR builds on the previous PRs and includes their changes so it would be wasteful to review it before the other stuff is merged and the amount of changes is reduced. I'll probably have to re-upload it then.
This PR moves the handling of field of fire sprites out of BoardView into its own handler that is no longer a mandatory part of the BoardView. Managing that handler is done in ClientGUI only. The *Displays call ClientGUI methods to pass on changes in the field of fire such as position, facing etc.