MegaMek / mekhq

MekHQ is a java helper program for the MegaMek game that allows users to load a list of entities from an XML file, perform repairs and customizations, and then save the new entities to another XML file that can be loaded into MegaMek.
http://megamek.org
134 stars 171 forks source link

[0.49.19 Nightly] The "Resolve Manually" function is incorrectly marking some units as "escaped" even though they were destroyed #3999

Open PhoenixHeart512 opened 7 months ago

PhoenixHeart512 commented 7 months ago

Environment

megamek.MegaMek.initializeLogging(MegaMek.java:118) - Starting MegaMek v0.49.19-SNAPSHOT Build Date: 2024-04-15T03:28:17.547186100 Today: 2024-04-15 Origin Project: MegaMek Java Vendor: Eclipse Adoptium Java Version: 17.0.6 Platform: Windows 10 10.0 (amd64) System Locale: en_US Total memory available to MegaMek: 8 GB

Description

I am getting different results between the normal post-game wrap up and when I "manually resolve" the scenario using the same salvage file.

I'm not sure if this is a problem with how MegaMek is saving the file, or if it's a problem with how MekHQ is reading the file. Submitting this under mekHQ for now since that's where the issue manifests.

This is when using the auto-popup after getting back to mekHQ and appears correct: image

This is after I hit "cancel" on the image above and then manually resolved using the saved .mul salvage file: image

Additionally, the Goliath was an allied unit (Allied Ace scenario modifier) and should not be in the salvage list.

It is also reporting different levels of damage on my own units, though I did not grab screenshots of that behavior. An extra Cardinal was marked as "crippled" when I manually resolved compared to the automated pop-up.

Files

The zip file has a matching folder structure to a MekHQ installation. If you unzip the archive on top of an existing mekHQ installation it will put all the saves and custom units into the appropriate folders to load the campaign.

The campaign file has the units assigned to the scenario ("Engage - Heavy"), and the MegaMek save file included is during the physical phase of the final turn for quick access to the end-of-scenario popups.

Manually Resolve Bug.zip

Sleet01 commented 7 months ago

There's good news and bad news.

The good news: it should be possible to clean up the code used to resolve a scenario and re-use much more of it in both the post-game and the MUL-based resolutions. It may even be possible to bring them into perfect unison without making any changes to the .MUL file format.

The bad news: it's more work than can be done in one night, so properly this should go into .20.

There are several issues with the current .MUL file processing:

  1. In order to fill in missing information normally provided by the game object after finishing a game, the .MUL resolver pulls data from the campaign object. But this data is not guaranteed to be correct, and in fact may be changed by the game ending! The current state of the campaign should not be used to guess different entities' status, especially since some of that can be teased out of the .MUL file with some processing.
  2. There was a plan to do more post-processing on entities after loading the .MUL file but before presenting the resolution dialog... but it was never implemented.
  3. The .MUL file can't differentiate allied and enemy salvage; both go under the same tag. We should probably skip any salvage that doesn't appear in the <killed></killed> tag, but it might be cleaner to have a separate <cannotSalvage></cannotSalvage> tag (this might be useful for special scenarios as well, to prevent players taking too much salvage).
  4. Some of the incorrect status information such as "crippled" may be coming out of the say that we generate new TestUnit and UnitStatus objects from MUL entries; this could use more testing and validation.

At the very least we need to make changes to src/mekhq/campaign/ResolveScenarioTracker.java > loadUnitsAndPilots, and likely to that class's constructor as well. MUL updates should probably be avoided unless absolutely necessary.

Thom293 commented 7 months ago

If you are looking at this, I'll throw these in here or can do a separate RFE if preferred.

  1. Like your "cannot be salvaged" flag, a "cannot be captured" or "always escapes" flag for pilots would be very useful for having a "persistent bad guy" or ally. You could have him show up in successive story missions without the worry that he was captured earlier. EDIT: I perhaps should suggest this in story arcs too but will leave here for an idea.
  2. If you give control of your units to the bot and run the game, in that one circumstance it does not ask you if you want to save a mul at the end of game. It would be useful if it always asked you at end of game, regardless of circumstances, I think.

Again if these don't belong here I can do a separate rfe. Just let me know.

IllianiCBT commented 7 months ago

I run a lot of Bot v. Bot games when testing, Thom’s second point would be an amazing time saver as currently if I want to re-test the same fight I have to run it multiple times.

To add onto his requests, we have a persistent bug where picking up an enemy pilot causes them to vanish from the Scenario Resolution dialog. Would be pretty cool if, while implementing your fixes, you could take a look into that, too.

Sleet01 commented 7 months ago

I think this is fine as an issue; these really are more bug fixes than new feature requests.

To add onto his requests, we have a persistent bug where picking up an enemy pilot causes them to vanish from the Scenario Resolution dialog. Would be pretty cool if, while implementing your fixes, you could take a look into that, too.

There's already code to separate out entities carried by other units; a quick fix might be making captured / picked-up pilots actually considered "passengers" of the unit that collects them. Or, again, add a new MUL tag - adding tags is less likely to hurt compatibility than removing them.

If you give control of your units to the bot and run the game, in that one circumstance it does not ask you if you want to save a mul at the end of game. It would be useful if it always asked you at end of game, regardless of circumstances, I think.

One would expect that, as long as there is one human attached to the game, they should get that prompt.