Scenario loader refactor & bugfix #117

Akjosch · 2016-03-31T12:46:58Z

This fixes most of the remaining problems in SF bug 3189 (Various Scenario Issues), aside of the inability to set ammo type. I'd like to add this too, but it would be helpful if someone would offer a bit of guidance how to query and verify ammo. Something about EquipmentType.get(ammoString), maybe, and then ... what?

(Tagged "pending" until that's cleared up, implemented, and tested)

…me key)

arlith · 2016-03-31T13:31:21Z

Verify ammo in what way? I think that you can probably find code examples for what you need in EquipChoiceDialog (in the megamek.client.ui.swing package I believe) and EquipChoiceDialog.MunitionsPanel. That's off the top of my head, so it may be slightly off. But, that's where munition switching is handled in the lobby, so I'd suspect you can find most of what you need there.

arlith · 2016-03-31T13:32:09Z

(In general, the scenarios for MM is something I've wanted to update for a long time, but never really got the time to do.)

nderwin · 2016-03-31T13:34:37Z

A couple of style comments - because I can't comment directly on ScenarioLoader because GitHub won't show the diff... grr GitHub...

there's some tab characters in the file, those should be replaced with spaces
go ahead and use the diamond operator on all those places that were changed that are using generics (Vector -> List, etc).
the make sure all the inner classes are grouped together and there's not any methods of the main class interspersed; also, if those are really only used by the main class, consider making them private or package protected

Curious - did you notice any performance improvement from replacing the vectors with array lists?

Akjosch · 2016-03-31T13:51:20Z

Oh FFS, stupid tabs ... How did those end up in there again?

Anyway, performance wasn't really my concern, given how tiny those files are. It's hard to say either way - on the one hand, the code is not doing any synchronised calls, on the other hand doing things the generic way tends to be slightly slower, on the other other hand, there's a lot of other things going on there like getting rid of implicit StringBuilder usage, using Pattern, using String.split() instead of StringTokenizer, so if that is ever a concern, only profiling would help.

Akjosch · 2016-03-31T16:04:33Z

Ok, added a simple variant using the equipment names in EquipmentType.lookupHash - I need to check all the game settings like EquipChoiceDialog, but that should do for now. The original code for setting the ammo amount was limited to 'Mechs only anyway ...

Syntax (note to myself: Update the example file accordingly):

 Unit_Faction_Number_SetAmmoType=Slot-AmmoName

arlith · 2016-03-31T16:51:46Z

Looking at the code in MulParser may also be useful too.

Akjosch · 2016-03-31T20:52:06Z

Just checked MULParser, and it only cares if it's AmmoType in there. If it is, the parser is totally fine with the LB-X 10 ammo slot being loaded with SRM4 Inferno ammo.

… weapon's ammo type as well, if there is one

arlith · 2016-04-01T02:05:06Z

Cool. Heh. Probably wouldn't hurt to have some sanity checking there...

Akjosch · 2016-04-01T10:48:10Z

Added legality checks. Reporting to log, looks like follows:

Ammo Flechette AC/10 Ammo (TL 1) is not legal for year 3150 (TL 0)
Illegal ammo type 'ISAC10 Flechette Ammo' for unit Hatchetman HCT-3F (Viper), slot AC/10 Ammo

It can even load the 4th scenario for the Sommerset Strikers, which the current version can't (because 2:3 isn't a valid slot on the Centurion CN9-D and it hits a NPE there).

arlith · 2016-04-01T12:50:54Z

Wonder if the CN9-D thing happened after a unit file changed. Probably would be worth having a unit test that at least checks to see if the scenarios can be loaded, just so we can ensure that the scenarios MM ships with are going to work.

Akjosch · 2016-04-01T12:54:39Z

Well, I guess I can quickly add a test class to the whole thing.

Akjosch · 2016-04-01T13:52:26Z

Current output of the tester:

ERROR in data\scenarios\1stSomersetStrikers\4-RaceForTheKwaidan.mms
Centurion CN9-D - invalid slot specified 2: 3
ERROR in data\scenarios\1stSomersetStrikers\9-MalthusAttacks.mms
megamek.server.ScenarioLoader$ScenarioLoaderException: Scenario requires missing entity: Sloth Battle Armor
    at megamek.server.ScenarioLoader.parseEntityLine(ScenarioLoader.java:537)
    at megamek.server.ScenarioLoader.buildFactionEntities(ScenarioLoader.java:443)
    at megamek.server.ScenarioLoader.createGame(ScenarioLoader.java:390)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:67)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:86)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:86)
    at megamek.test.ScenarioLoadierTest.runTests(ScenarioLoadierTest.java:56)
    at megamek.test.ScenarioLoadierTest.main(ScenarioLoadierTest.java:48)
ERROR in data\scenarios\Lucho\Citadel_Blitz_multiplayer.mms
megamek.server.ScenarioLoader$ScenarioLoaderException: Scenario requires missing entity: false
    at megamek.server.ScenarioLoader.parseEntityLine(ScenarioLoader.java:537)
    at megamek.server.ScenarioLoader.buildFactionEntities(ScenarioLoader.java:443)
    at megamek.server.ScenarioLoader.createGame(ScenarioLoader.java:390)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:67)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:86)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:86)
    at megamek.test.ScenarioLoadierTest.runTests(ScenarioLoadierTest.java:56)
    at megamek.test.ScenarioLoadierTest.main(ScenarioLoadierTest.java:48)
ERROR in data\scenarios\Lucho\Citadel_Blitz_oneonone.mms
megamek.server.ScenarioLoader$ScenarioLoaderException: Scenario requires missing entity: false
    at megamek.server.ScenarioLoader.parseEntityLine(ScenarioLoader.java:537)
    at megamek.server.ScenarioLoader.buildFactionEntities(ScenarioLoader.java:443)
    at megamek.server.ScenarioLoader.createGame(ScenarioLoader.java:390)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:67)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:86)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:86)
    at megamek.test.ScenarioLoadierTest.runTests(ScenarioLoadierTest.java:56)
    at megamek.test.ScenarioLoadierTest.main(ScenarioLoadierTest.java:48)
ERROR in data\scenarios\Lucho\Last_Stand_at_Guanabara.mms
megamek.server.ScenarioLoader$ScenarioLoaderException: Scenario requires missing entity: true
    at megamek.server.ScenarioLoader.parseEntityLine(ScenarioLoader.java:537)
    at megamek.server.ScenarioLoader.buildFactionEntities(ScenarioLoader.java:443)
    at megamek.server.ScenarioLoader.createGame(ScenarioLoader.java:390)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:67)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:86)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:86)
    at megamek.test.ScenarioLoadierTest.runTests(ScenarioLoadierTest.java:56)
    at megamek.test.ScenarioLoadierTest.main(ScenarioLoadierTest.java:48)
ERROR in data\scenarios\Northwind Highlanders\NWH2_InstantFame.mms
Equality sign in scenario file data\scenarios\Northwind Highlanders\NWH2_InstantFame.mms on line 10 missing; ignoring
Equality sign in scenario file data\scenarios\Northwind Highlanders\NWH2_InstantFame.mms on line 11 missing; ignoring
Equality sign in scenario file data\scenarios\Northwind Highlanders\NWH2_InstantFame.mms on line 12 missing; ignoring
ERROR in data\scenarios\TheGreatRefusal\1-SmokeJaguarVsSteinerDavion.mms
megamek.server.ScenarioLoader$ScenarioLoaderException: Scenario requires missing entity: Daishi (Dire Wolf) "Prometheus"
    at megamek.server.ScenarioLoader.parseEntityLine(ScenarioLoader.java:537)
    at megamek.server.ScenarioLoader.buildFactionEntities(ScenarioLoader.java:443)
    at megamek.server.ScenarioLoader.createGame(ScenarioLoader.java:390)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:67)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:86)
    at megamek.test.ScenarioLoadierTest.checkScenarioFile(ScenarioLoadierTest.java:86)
    at megamek.test.ScenarioLoadierTest.runTests(ScenarioLoadierTest.java:56)
    at megamek.test.ScenarioLoadierTest.main(ScenarioLoadierTest.java:48)

The "Scenario requires missing entity: false" errors are due to lines like Unit_<Faction>_AutoEject=false (instead of Unit_<Faction>_<Something>_AutoEject=false). That's the wrong syntax for faction-wide settings anyway; those are formed like AutoEject_<Faction>=false (though this one doesn't exist yet).

The Northwind Highlanders scenario simply has a multi-line description ...

(Now I need to see how I can make it into a "should not output anything" unit test ...)

Akjosch · 2016-04-01T17:20:04Z

So, yeah. With the scenarios fixed and all loading, this should be about done. Unless somebody has some other important wishes?

arlith · 2016-04-01T18:50:08Z

Well, if you're asking for wishes....

Oh, you mean related to this PR? Nah, seems good to me.

HammerGS · 2016-04-01T19:23:13Z

There where some requests on the forum about the files;

Is there a way to set an aerospace units initial altitude in a scenario file? In-particular I'd like to be able to set them as landed.

C3 network when playing from a scenario

Given a target
http://megamek.info/forums/index.php?topic=1001.msg6462#msg6462

Creating file.
http://megamek.info/forums/index.php?topic=1239.msg13583#msg13583

Just to mention a few

Akjosch · 2016-04-01T20:17:42Z

Giving an altitude is easy (done).

Setting up C3 network ... isn't. This stuff needs quite a bit of validation post-unit initialisation and has lots of options depending on which network layout people are trying to set up - and some way to clearly communicate errors.

I've yet to check how MM handles objectives, so I'd rather leave that out for now.

The scenario in that thread actually loads fine ... The Dixie Militia use a non-ASCII character in there (I think it's an em-dash), which might have something to do with it, but I think switching the parsing over to regular expressions killed whatever bug prevented it from loading.

Akjosch added 9 commits March 30, 2016 23:33

Replacing vectors

a1d6fd7

Slight cleanup

8443d5b

Kicking out Property (it doesn't work with multiple values for the sa…

1bef1fe

…me key)

Refactoring entity parsing

d678806

Refactoring player parsing

bb6f0e0

Refactoring board parsing

4ac93be

Refactoring damage application

de1f283

I18N

50f03c5

Don't mess with player order

26ebf15

Akjosch added the pending label Mar 31, 2016

Slight updates to the unit data pattern matchers

dc1fdbe

Style fixes

70b5b10

Removing System.err (System.out is enough for logging)

23d85d2

Adding Unit_<Faction>_<Whatever>_SetAmmoType=<Slot>-<AmmoType> setting

6f31bbb

Guarding against specifying non-ammo as ammo; checking against linked…

cd6f351

… weapon's ammo type as well, if there is one

Checks for legality according to game rules

cc86eae

Akjosch removed the pending label Apr 1, 2016

Guarding against illegal ammo slots

222e7c0

Adding scenario loader tester

e2d152b

Akjosch added 5 commits April 1, 2016 16:09

Moving all test code into the instance method

eadb1a4

Make runTests() return a list with errors instead of printing them

0b64ae5

Tiny change in test class: print errors AND record them.

0c0342a

Fixing scenarios

0844b48

Fixing minor bugs, NPE guards, updating example file

a036ac0

Allow setting of altitude

b41a9dd

Akjosch merged commit 8b12995 into MegaMek:master Apr 2, 2016

Akjosch deleted the scenario_loader_refactor branch April 2, 2016 08:10

PhoenixHeart512 mentioned this pull request Jun 14, 2023

0.49.14 - Uncaught Exception after Physical Attack phase #4523

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scenario loader refactor & bugfix #117

Scenario loader refactor & bugfix #117

Akjosch commented Mar 31, 2016

arlith commented Mar 31, 2016

arlith commented Mar 31, 2016

nderwin commented Mar 31, 2016

Akjosch commented Mar 31, 2016

Akjosch commented Mar 31, 2016

arlith commented Mar 31, 2016

Akjosch commented Mar 31, 2016

arlith commented Apr 1, 2016

Akjosch commented Apr 1, 2016

arlith commented Apr 1, 2016

Akjosch commented Apr 1, 2016

Akjosch commented Apr 1, 2016

Akjosch commented Apr 1, 2016

arlith commented Apr 1, 2016

HammerGS commented Apr 1, 2016

Akjosch commented Apr 1, 2016

Scenario loader refactor & bugfix #117

Scenario loader refactor & bugfix #117

Conversation

Akjosch commented Mar 31, 2016

arlith commented Mar 31, 2016

arlith commented Mar 31, 2016

nderwin commented Mar 31, 2016

Akjosch commented Mar 31, 2016

Akjosch commented Mar 31, 2016

arlith commented Mar 31, 2016

Akjosch commented Mar 31, 2016

arlith commented Apr 1, 2016

Akjosch commented Apr 1, 2016

arlith commented Apr 1, 2016

Akjosch commented Apr 1, 2016

Akjosch commented Apr 1, 2016

Akjosch commented Apr 1, 2016

arlith commented Apr 1, 2016

HammerGS commented Apr 1, 2016

Akjosch commented Apr 1, 2016