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

Fix 4754 vtols and cvs missing troop space bays #4760

Merged

Conversation

Sleet01
Copy link
Collaborator

@Sleet01 Sleet01 commented Sep 8, 2023

Fixes:

  1. An off-by-one error I introduced, which prevents units with single bays (usually troop/infantry) from populating.
  2. 55 or so BLK files with multiple <transporters> blocks that wouldn't have loaded correctly anyhow - only the first can be read.
  3. Corrected several vehicles where multiple bays had been added, or the top bay was a correction for the lower bay, or (especially in the case of the Ajax D) bays, weapons, and equipment were incorrect.
  4. Reverted the change that is causing MML to crash in the cache view, so that I could use it to correct 2 and 3 above.

Testing:

  • loaded all of the vehicles with corrected bays in MML, with a sampling also loaded in MM, to verify all loadouts are now valid.
  • loaded vehicles in MM and populated troop bays / BA handholds with various infantry and BA units:
image

close #4754

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Original dev is aware that this needs fixing, but I need MML for other work, so I'd like to revert this change until the permanent fix is ready.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd initially read these functions as finding the start and end tags for a given block; instead they find the line index of the first text line, and the line index after the last text line, so e.g.:

<transporters>
troopspace:4.0
</transporters>

would give an index difference of 1, and

<transporters>
</transporters>

would give a difference of 0, rather than 1 as I had initially assumed. Mea culpa.

@@ -37,11 +41,7 @@ Hover
</motion_type>

<transporters>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I read this as a botched correction, not as an attempt to create two separate bays. Updating to a single bay with the new size produced entries that match the TRO.

@@ -41,8 +41,9 @@ Hover
</motion_type>

<transporters>
troopspace:8.0
BattleArmorHandles - troopers:-1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I only updated items to "OMNI" when I could confirm them via the TRO or sheet.

@SJuliez SJuliez requested a review from HammerGS September 8, 2023 13:47
@Sleet01
Copy link
Collaborator Author

Sleet01 commented Sep 8, 2023

Sorry, I just spotted the merge conflict; if the new version is working, my reversion is no longer necessary.

@SJuliez SJuliez merged commit d591e9b into MegaMek:master Sep 13, 2023
4 checks passed
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.

[0.49.15-SNAPSHOT] Vehicles seem not to be registering troopspace correctly including DI Multipurpose VTOL
2 participants