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.

Sign up for GitHub

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

Jump to bottom

Aero/Squadrons: various changes #4800

Merged
merged 3 commits into from
Sep 27, 2023
Merged

Aero/Squadrons: various changes #4800

merged 3 commits into from
Sep 27, 2023

Conversation

SJuliez
Copy link
Member

@SJuliez SJuliez commented Sep 20, 2023

This PR makes the following changes:

  • No longer shows TMM pips for Aero units
  • No longer shows "Internal" in the unit tooltip for fighter squadrons
  • Treats customizing a fighter squadron in the lobby as a multi-unit custom, so pilot, quirks etc can no longer be changed
  • On the in-game unit sprite, removes the [T] indicator for transporting other units from fighter squadrons as they only transport their own fighters
  • On the in-game unit sprite, removes the internal structure bar for fighter squadrons as they don't suffer SI/Internalstruc damage
  • In the lobby, no longer shows a role for fighter squadrons (it was always "undetermined")
  • On the minimap of a space map, fades out stars and hex borders somewhat at low zoom (small minimap) so they don't overwhelm the image as much
  • Increases randomness for the image choice for the base image of hexes; this removes the stripy-ness of the space map (it is still deterministic and should not change appearance randomly)
  • Allows loading fighter squadrons onto carriers; sadly this is still limited to ASF bays that are at least as big as the whole squadron, so for a Leopard CV only squadrons of at most 2 fighters can be loaded; distributing an fsq into multiple bays would require quite a bit more coding as the current transporters don't support that; the ugly part of this change are the additions in GameManager that deal with changing squadrons when they're carried
  • Allows calling up the "View" of a fighter squadron in the lobby (some guard code in the TROView)
  • And a little fun thing: fsq now construct their icon from the actual fighters:
    image image image image image

github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 20, 2023
View reviewed changes
megamek/src/megamek/client/ui/swing/tileset/HexTileset.java
@@ -218,7 +218,8 @@
}
}

Image img = bestMatch.getImage(comp, hex.getCoords().hashCode());
Random random = new Random(hex.getCoords().hashCode());
Image img = bestMatch.getImage(comp, Math.abs(random.nextInt() * random.nextInt()));

Check warning

Code scanning / CodeQL

Dereferenced variable may be null Warning

Variable
bestMatch
Loading
may be null at this access because of
this
Loading
assignment.
Copy link
Member

Choose a reason for hiding this comment

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

Is this null actually possible here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only when there are no base images in the tileset or it didnt load, so only when things are very bad. I'd say this can stay as it was (its the bestMatch.getImage() call and used to be like that before)

NickAragua
NickAragua requested changes Sep 21, 2023
View reviewed changes
Copy link
Member

@NickAragua NickAragua left a comment

Choose a reason for hiding this comment

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

Just a little cleanup.

megamek/src/megamek/client/ui/swing/boardview/EntitySprite.java Outdated
// Transporting
if (!entity.getLoadedUnits().isEmpty()) {
// Transporting (but not Squadrons that are obviously composed of subunits)
if (!entity.getLoadedUnits().isEmpty() && !(entity instanceof FighterSquadron)) {
Copy link
Member

Choose a reason for hiding this comment

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

You may as well use "isSquadron" here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep

megamek/src/megamek/client/ui/swing/tileset/HexTileset.java
@@ -218,7 +218,8 @@
}
}

Image img = bestMatch.getImage(comp, hex.getCoords().hashCode());
Random random = new Random(hex.getCoords().hashCode());
Image img = bestMatch.getImage(comp, Math.abs(random.nextInt() * random.nextInt()));
Copy link
Member

Choose a reason for hiding this comment

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

Is this null actually possible here?

@SJuliez
Copy link
Member Author

SJuliez commented Sep 21, 2023

Very sorry, this actually needed a bit more cleanup. I added positioning for squadrons of up to 10 fighters, disallowed icon export for fsq in the lobby with a specific camo (doesnt work with the new icons) and added a static constructor for EntityImage. Aaaah and I'll need to check if this affects MHQ. Not quite there yet!

@SJuliez
Copy link
Member Author

SJuliez commented Sep 21, 2023

OK, I added the constructors back in EntityImage for MHQ. Now it should be good (or at least better)

@NickAragua NickAragua merged commit 08c0839 into MegaMek:master Sep 27, 2023
4 checks passed
@SJuliez SJuliez deleted the Squadrons branch October 14, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NickAragua NickAragua NickAragua approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SJuliez @NickAragua