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

Steps towards SBF, player turns and stuff #5541

Merged

Conversation

SJuliez
Copy link
Member

@SJuliez SJuliez commented Jun 2, 2024

This PR is a further small step towards SBF game handling. While having to look closely at our classes, I am using the opportunity to do some refactoring (or better: reorganization), making our existing classes smaller and seeing how to reuse them for SBF games.
This PR mainly

  • adds a PlayerTurn interface to generalize player turns (GameTurn is specific to TW games)
  • moves out GameTurn's inner subclasses
  • moves up some general phase handling to AbstractGameManager and moves out the specific implementations into handler classes, also for GameManager (cutting its length by over 1000 lines ... i.e. around 3% lol); this requires some access modifier changes to methods in GameManager

As in the last PRs for SBF I ask to leave in the loads of commented code in the SBF classes so I don't have to re-copy them later to see what's to be done. There are no intentional changes for standard games in here.

For review, there is a lot of code movement in here, so it's not quite as much as it looks like. Still, it's another big one. To test it I had two Princesses have a royal rumble and it seemed to work well.

SJuliez added 2 commits June 4, 2024 17:19
…rocessing-NEW2

# Conflicts:
#	megamek/src/megamek/server/GameManager.java
#	megamek/src/megamek/server/SBFGameManager.java
Copy link

codecov bot commented Jun 8, 2024

Codecov Report

Attention: Patch coverage is 1.87919% with 731 lines in your changes missing coverage. Please review.

Project coverage is 29.29%. Comparing base (a88ea44) to head (ae6f1a2).
Report is 12 commits behind head on master.

Current head ae6f1a2 differs from pull request most recent head 3706e70

Please upload reports for the commit 3706e70 to get more accurate results.

Files Patch % Lines
megamek/src/megamek/server/TWPhaseEndManager.java 1.67% 176 Missing ⚠️
.../src/megamek/server/TWPhasePreparationManager.java 1.73% 170 Missing ⚠️
megamek/src/megamek/server/SBFPhaseEndManager.java 0.00% 54 Missing ⚠️
...src/megamek/server/SBFPhasePreparationManager.java 0.00% 44 Missing ⚠️
...amek/src/megamek/server/GameManagerSaveHelper.java 7.14% 39 Missing ⚠️
megamek/src/megamek/common/UnloadStrandedTurn.java 0.00% 37 Missing ⚠️
megamek/src/megamek/common/EntityClassTurn.java 0.00% 31 Missing ⚠️
...egamek/src/megamek/server/AbstractGameManager.java 6.66% 28 Missing ⚠️
megamek/src/megamek/server/SBFGameManager.java 0.00% 27 Missing ⚠️
megamek/src/megamek/common/Game.java 0.00% 18 Missing ⚠️
... and 28 more
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #5541   +/-   ##
=========================================
  Coverage     29.28%   29.29%           
- Complexity    13758    13771   +13     
=========================================
  Files          2443     2459   +16     
  Lines        262494   262554   +60     
  Branches      46974    46963   -11     
=========================================
+ Hits          76875    76917   +42     
- Misses       181766   181796   +30     
+ Partials       3853     3841   -12     

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

Copy link
Collaborator

@Sleet01 Sleet01 left a comment

Choose a reason for hiding this comment

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

Some naming questions, otherwise looks good (I didn't spend much time on SBF stuff as I believe it's still in flux).

@SJuliez SJuliez merged commit ca70d39 into MegaMek:master Jun 13, 2024
4 checks passed
@SJuliez SJuliez deleted the SBF-step-combine-reports-turn-processing-NEW2 branch June 14, 2024 13:48
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.

2 participants