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

Facilitating other game types: disconnecting Force and Entity #4094

Merged
merged 7 commits into from
Jan 15, 2023

Conversation

SJuliez
Copy link
Member

@SJuliez SJuliez commented Jan 8, 2023

This PR has no visible results but tries to form the code to facilitate other types of game than the TW game we have now for some unforeseeable future when someone wants to implement MegaSBF or something.

So, the main goal of this PR is to allow other types of unit than Entity to be held in an MM Force because a Force is mostly a list of IDs and there's no reason not to be able to put Alpha Strike Elements or anything else in it. To do this, an interface ForceAssignable is added. To give this a bit of useful structure and relieve the unit classes of utility method code while actually adding utility some more interfaces are added:

  • BTObject is the top-level interface applicable (ideally) to everything that can end up on the battlefield or represent something that goes on the battlefield, regardless of what type of game it is and whether there is a game. It has a bunch of utility methods like isFighter and IsIndustrialMek. This interface is, for now, implemented by Entity, ASE and MechSummary
  • InGameObject extends BTObject and requires the object to have an id. This is implemented by Entity and ASE (MechSummaries get no id and don't go on the battlefield)
  • Targetable was present before but now extends InGameObject. Targetable's previous getTargetID() and Entity's getID() were combined into InGameObject's getID() as I could not see a reason to keep them separate. This change makes up the bulk of file changes in this PR (it's not just a cheap rename, getTargetID() is removed entirely. Entity already returned its own getID() for getTargetID() and the other types of Targetable had no other ID than the targetID. There used to be nothing preventing overlapping ids for Targetables of different type, therefore my conclusion that I can unify them)
  • ForceAssignable also extends InGameObject and is implemented by Entity and ASE.

In addition, the IGame interface is slightly extended with a few methods that I expect every type of game would use and an AbstractGame base implementation to IGame is added that removes the unit, player and team list management from Game (which could be renamed TWGame but I kept myself from doing that for now). AbstractGame also makes use of the new interfaces by keeping not a Map of ID to Entity but ID to InGameObject.

Finally this removes a small piece of code from Team to make it more or less fully game-type agnostic. Also removed is some old Enumeration-style code.

My hope would be that while Game/GameManager are (and should remain) TW-specific, Player, Team, Force, ideally ChatLounge=Lobby, Server and (maybe) Princess should be game-type agnostic thus make no reference to Entity and Game, rather use InGameObject and IGame.

# Conflicts:
#	megamek/src/megamek/client/ui/swing/PlayerListDialog.java
#	megamek/src/megamek/common/Mech.java
#	megamek/src/megamek/common/SmallCraft.java
#	megamek/src/megamek/common/Tank.java
#	megamek/src/megamek/common/weapons/MissileWeaponHandler.java
@codecov
Copy link

codecov bot commented Jan 8, 2023

Codecov Report

Base: 22.95% // Head: 23.01% // Increases project coverage by +0.06% 🎉

Coverage data is based on head (ddb1cf7) compared to base (2eb2476).
Patch coverage: 4.44% of modified lines in pull request are covered.

❗ Current head ddb1cf7 differs from pull request most recent head a799bb8. Consider uploading reports for the commit a799bb8 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4094      +/-   ##
============================================
+ Coverage     22.95%   23.01%   +0.06%     
- Complexity     4816     4817       +1     
============================================
  Files          2277     2280       +3     
  Lines        250300   249555     -745     
  Branches      46373    46282      -91     
============================================
- Hits          57463    57444      -19     
+ Misses       191385   190660     -725     
+ Partials       1452     1451       -1     
Impacted Files Coverage Δ
megamek/src/megamek/client/Client.java 5.09% <0.00%> (-0.03%) ⬇️
megamek/src/megamek/client/bot/MoveOption.java 0.00% <0.00%> (ø)
megamek/src/megamek/client/bot/PhysicalOption.java 0.00% <0.00%> (ø)
...ek/src/megamek/client/bot/princess/FiringPlan.java 52.59% <0.00%> (ø)
...ek/client/bot/princess/MultiTargetFireControl.java 0.00% <0.00%> (ø)
...ent/bot/princess/NewtonianAerospacePathRanker.java 0.00% <0.00%> (ø)
...ek/src/megamek/client/bot/princess/PathRanker.java 29.76% <0.00%> (ø)
.../src/megamek/client/bot/princess/PhysicalInfo.java 55.76% <0.00%> (ø)
...amek/src/megamek/client/bot/princess/Princess.java 17.79% <0.00%> (ø)
...rc/megamek/client/bot/princess/WeaponFireInfo.java 35.76% <0.00%> (ø)
... and 118 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

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 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@SJuliez SJuliez merged commit 7736c49 into MegaMek:master Jan 15, 2023
@SJuliez SJuliez deleted the ForceAssignable branch January 16, 2023 20:33
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