Page MenuHomeWildfire Games

Improve and fix checkrefs.pl
ClosedPublic

Authored by Stan on Mar 12 2018, 1:42 AM.

Details

Summary

That perl script is a great tool, that should probably be included in our tests. Currently it's a bit broken due to a lot of changes in our codebase.

if (perlscriptshouldbecomepython)
    return;

This patches fixes variant support
and aims when it's done to add multi-mod support.

This patches fixes the following errors
Loading GUI XML...

Can't stat ../../../binaries/data/mods/public/gui/modmod: No such file or directory
 at checkrefs.pl line 60.
Loading random maps...
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
`

Adds command line arguments

'--check-map-xml     
'--check-unused'      
'--validate-templates'
'--root-actors'
'--mod-to-check=s'

Removes a noisy print

Test Plan

test it no longer creates an incorrect report.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 5780
Build 9693: Vulcan BuildJenkins

Event Timeline

Stan created this revision.Mar 12 2018, 1:42 AM
Vulcan added a subscriber: Vulcan.Mar 12 2018, 2:45 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/differential/237/display/redirect

elexis added a subscriber: elexis.Apr 7 2018, 10:53 AM

The feature description could be a bit more precise. For instance it could show the error messages that it currently throws that this patch fixes (by looking up files in the mod mod as well which is currently missing I suppose?)

(I really don't like the consequent condition if-statement style, but the rest of the code does it too.)

Also the patch should not be relative to source/.

That's the output of checkrefs without unused entries for current svn r21666:

Duplicate terrain name 'medit_city_tile' (from 'art/terrains/road/medit_city_tile.xml' and 'art/terrains/biome-mediterranean/medit_city_tile.xml')
Loading entities...
Loading actors...
Loading art files...
Loading materials...
Loading particles...
Loading sound groups...
Loading audio files...
Loading GUI XML...
Can't stat ../../../binaries/data/mods/public/gui/modmod: No such file or directory
 at checkrefs.pl line 50.
Loading GUI data...
Loading civs...
Loading random maps...
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.
Loading techs...
Loading terrains...

Missing file 'art/textures/ui/session/icons/mappreview/sudd.png' referenced by: 'public/maps/random/sudd.json'
Missing file 'art/textures/ui/session/portraits/' referenced by: 'public/simulation/templates/special/filter/mirage.xml'
Missing file 'maps/random/' referenced by: 'public/maps/random/hellas_biomes.json', 'public/maps/random/rmbiome/defaultbiome.json', 'public/maps/random/rmbiome/fields_of_meroe/dry.json', 'public/maps/random/rmbiome/fields_of_meroe/rainy.json', 'public/maps/random/rmbiome/generic/alpine.json', 'public/maps/random/rmbiome/generic/autumn.json', 'public/maps/random/rmbiome/generic/desert.json', 'public/maps/random/rmbiome/generic/mediterranean.json', 'public/maps/random/rmbiome/generic/savanna.json', 'public/maps/random/rmbiome/generic/snowy.json', 'public/maps/random/rmbiome/generic/temperate.json', 'public/maps/random/rmbiome/generic/tropic.json', 'public/maps/random/rmbiome/gulf_of_bothnia/frozen_lake.json', 'public/maps/random/rmbiome/gulf_of_bothnia/late_spring.json', 'public/maps/random/rmbiome/gulf_of_bothnia/winter.json', 'public/maps/random/rmbiome/persian_highlands/spring.json', 'public/maps/random/rmbiome/persian_highlands/summer.json'
Missing file 'maps/random/sudd.js' referenced by: 'public/maps/random/sudd.json'

This is what I get after applying the patch:

Loading maps XML...
  maps/scenarios/Arcadia.xml
  maps/scenarios/Azure Coast(2).xml
  maps/scenarios/Azure Coast(4).xml
  maps/scenarios/Azure Coast.xml
  maps/scenarios/Battle for the Tiber.xml
  maps/scenarios/Belgian_Bog_night.xml
  maps/scenarios/Bridge_demo.xml
  maps/scenarios/Campaign Test Map 2 - heightmap.xml
  maps/scenarios/Campaign Test Map.xml
  maps/scenarios/Cinema_Demo.xml
  maps/scenarios/Combat_demo.xml
  maps/scenarios/Combat_demo_(huge).xml
  maps/scenarios/Death Canyon - Invasion Force.xml
  maps/scenarios/Demo_Trading.xml
  maps/scenarios/Eire and Albion.xml
  maps/scenarios/Fast Oasis.xml
  maps/scenarios/Fishing_demo.xml
  maps/scenarios/Flight_demo.xml
  maps/scenarios/Flight_demo_2.xml
  maps/scenarios/Gold_Rush.xml
  maps/scenarios/Gorge.xml
  maps/scenarios/Height Map Import - Demo (Fractal).xml
  maps/scenarios/Height Map Import - Demo (Greece).xml
  maps/scenarios/Height Map Import - Demo (Greece-Small).xml
  maps/scenarios/Laconia 01.xml
  maps/scenarios/Migration.xml
  maps/scenarios/Miletus.xml
  maps/scenarios/Multiplayer_demo.xml
  maps/scenarios/Napata Reconstruction.xml
  maps/scenarios/Necropolis.xml
  maps/scenarios/Pathfinding_demo.xml
  maps/scenarios/Pathfinding_terrain_demo.xml
  maps/scenarios/Peloponnese.xml
  maps/scenarios/Polynesia.xml
  maps/scenarios/Resource_demo.xml
  maps/scenarios/Saharan Oases.xml
  maps/scenarios/Sahel.xml
  maps/scenarios/Sandbox - Athenians.xml
  maps/scenarios/Sandbox - Britons.xml
  maps/scenarios/Sandbox - Carthaginians.xml
  maps/scenarios/Sandbox - Gauls.xml
  maps/scenarios/Sandbox - Iberians.xml
  maps/scenarios/Sandbox - Kushites.xml
  maps/scenarios/Sandbox - Macedonians.xml
  maps/scenarios/Sandbox - Mauryans.xml
  maps/scenarios/Sandbox - Persians.xml
  maps/scenarios/Sandbox - Ptolemies 2.xml
  maps/scenarios/Sandbox - Ptolemies.xml
  maps/scenarios/Sandbox - Romans.xml
  maps/scenarios/Sandbox - Seleucids.xml
  maps/scenarios/Sandbox - Spartans.xml
  maps/scenarios/Savanna Ravine.xml
  maps/scenarios/Serengeti.xml
  maps/scenarios/Ship Formations.xml
  maps/scenarios/Siwa Oasis.xml
  maps/scenarios/Territory Demo.xml
  maps/scenarios/The Massacre of Delphi.xml
  maps/scenarios/The Persian Gates.xml
  maps/scenarios/Third Macedonian War.xml
  maps/scenarios/Treasure Islands.xml
  maps/scenarios/Triggers_demo.xml
  maps/scenarios/Tropical Island.xml
  maps/scenarios/Units_demo.xml
  maps/scenarios/WallTest.xml
  maps/scenarios/Walls.xml
  maps/scenarios/We are Legion.xml
  maps/scenarios/_default.xml
  maps/scenarios/auratest.xml
  maps/scenarios/elephantine_environment2.xml
  maps/scenarios/jebel_barkal.xml
  maps/scenarios/oos_modificationtemplates_onefemale.xml
  maps/scenarios/oos_theatron.xml
  maps/scenarios/reservoir.xml
  maps/scenarios/road demo.xml
  maps/scenarios/shipattacks.xml
  maps/scenarios/temperate map.xml
  maps/skirmishes/Acropolis Bay (2).xml
  maps/skirmishes/Alpine_Mountains_(3).xml
  maps/skirmishes/Alpine_Valleys_(2).xml
  maps/skirmishes/Bactria (2).xml
  maps/skirmishes/Barcania (3).xml
  maps/skirmishes/Belgian Bog (2).xml
  maps/skirmishes/Butana Steppe (2).xml
  maps/skirmishes/Caspian Sea (2v2).xml
  maps/skirmishes/Corinthian Isthmus (2).xml
  maps/skirmishes/Corinthian Isthmus (4).xml
  maps/skirmishes/Corsica and Sardinia (4).xml
  maps/skirmishes/Cycladic Archipelago (2).xml
  maps/skirmishes/Cycladic Archipelago (3).xml
  maps/skirmishes/Death Canyon (2).xml
  maps/skirmishes/Deccan Plateau (2).xml
  maps/skirmishes/Dueling Cliffs (3v3).xml
  maps/skirmishes/Egypt (3v3).xml
  maps/skirmishes/Forest Battle (4).xml
  maps/skirmishes/Gallic Fields (3).xml
  maps/skirmishes/Gambia River (3).xml
  maps/skirmishes/Golden Island (2).xml
  maps/skirmishes/Golden Oasis (2).xml
  maps/skirmishes/Greek Acropolis (2).xml
  maps/skirmishes/Greek Acropolis (4).xml
  maps/skirmishes/Greek Acropolis Night (2).xml
  maps/skirmishes/Libyan Oases (4).xml
  maps/skirmishes/Libyan Oasis (2).xml
  maps/skirmishes/Lorraine Plain (2).xml
  maps/skirmishes/Median Oasis (2).xml
  maps/skirmishes/Median Oasis (4).xml
  maps/skirmishes/Mediterranean Cove (2).xml
  maps/skirmishes/Neareastern Badlands (2).xml
  maps/skirmishes/Neareastern Badlands (4).xml
  maps/skirmishes/Nile River (4).xml
  maps/skirmishes/Northern Island (2).xml
  maps/skirmishes/Persian Highlands (4).xml
  maps/skirmishes/Punjab (2).xml
  maps/skirmishes/Saharan Oases (4).xml
  maps/skirmishes/Sahel (4).xml
  maps/skirmishes/Savanna River.xml
  maps/skirmishes/Sicilia (2).xml
  maps/skirmishes/Sicilia_Nomad.xml
  maps/skirmishes/Siwa Oasis (2).xml
  maps/skirmishes/Skirmish Demo.xml
  maps/skirmishes/Sporades Islands (2).xml
  maps/skirmishes/Syria (2).xml
  maps/skirmishes/Team Oasis - 2v2.xml
  maps/skirmishes/Thessalian Plains (4).xml
  maps/skirmishes/Tuscan Acropolis (4).xml
  maps/skirmishes/Two Seas (6).xml
  maps/skirmishes/Via Augusta (4).xml
  maps/skirmishes/Watering Holes (4).xml
  maps/skirmishes/Zagros Mountains (2).xml
Loading maps PMP...
Loading entities...
Loading actors...
Loading variants...
Loading art files...
Loading materials...
Loading particles...
Loading sound groups...
Loading audio files...
Loading GUI XML...
Loading GUI data...
Loading civs...
Loading random maps...
Loading techs...
Loading terrains...



Validating actors...

0 actor validation errors

Validating variants...

0 variant validation errors

Validating gui pages...

0 gui page validation errors

Validating gui xmls...

0 gui xml validation errors

Validating maps...

0 map validation errors

Validating materials...

0 material validation errors

Validating particles...

0 particle validation errors

Validating pathfinders...

0 pathfinder validation errors

Validating territory managers...

0 territory manager validation errors

Validating sound groups...

0 sound group validation errors

Validating terrains...

0 terrain validation errors

Validating terrain textures...

0 terrain texture validation errors

Validating textures...

0 texture validation errors
Duplicate terrain name 'medit_city_tile' (from 'art/terrains/road/medit_city_tile.xml' and 'art/terrains/biome-mediterranean/medit_city_tile.xml')
Missing file 'art/textures/ui/session/icons/mappreview/sudd.png' referenced by: 'public/maps/random/sudd.json'
Missing file 'art/textures/ui/session/portraits/' referenced by: 'public/simulation/templates/special/filter/mirage.xml'
Missing file 'maps/random/sudd.js' referenced by: 'public/maps/random/sudd.json'
Missing file 'simulation/templates/uncapturable|structures/kush_defense_tower.xml' referenced by: 'public/maps/scenarios/elephantine_environment2.xml'
Missing file 'simulation/templates/uncapturable|structures/kush_sentry_tower.xml' referenced by: 'public/maps/scenarios/elephantine_environment2.xml'

sudd.js is about to be fixed by us.
The elephantine_environment2.xml entry shows that we need support for special filter templates, already reported at #4560.

I would suggest to do the following: Tell me what the mod mod change was needed for, then I'll likely accept this revision, you then remove the print warning, don't set the 0 constants to 1 and commit it excluding the variants change and then the variants change separately.

tools/entity/Entity.pm
143 ↗(On Diff #6155)

good

tools/entity/checkrefs.pl
10 ↗(On Diff #6155)

Not sure if this should be changed to 1, it takes very long.
These should become commandline arguments.

35 ↗(On Diff #6155)

Good to remove this unneeded var for now.

59 ↗(On Diff #6155)

What was this needed for?

233 ↗(On Diff #6155)

this print is noisy and inconsistent, should be deleted

445 ↗(On Diff #6155)

This fixes a bug currently, see output.
It must be the new JSON files that aren't maps. I guess the biomes.
checkrefs.log:Use of uninitialized value in concatenation (.) or string at checkrefs.pl line 444.

585 ↗(On Diff #6155)

This tests if the filepath appears anywhere, but why not further test that it appears in the right place?
i.e. instead of == -1 it should be an != with the number where the path starts.
(If possible each condition on one line, so that the symmetry immediately reveals that it's thrice the same condition with a different argument.)

tools/entity/entvalidate.pl
8 ↗(On Diff #6155)

Ehm, entity.rng does not exist, entire file fails?! File never existed? https://code.wildfiregames.com/source/0ad/browse/ps/trunk/binaries/system/entity.rng

That's my entire output of the tool:
Relax-NG parser error : xmlRelaxNGParse: could not load ../../../binaries/system/entity.rng

Stan updated this revision to Diff 6333.Apr 7 2018, 11:33 AM
Stan edited the summary of this revision. (Show Details)
Stan added a reviewer: elexis.
Stan marked 2 inline comments as done.

Fix above comments

tools/entity/checkrefs.pl
59 ↗(On Diff #6155)

See the first warning in the description (The one about not being able to stat)

445 ↗(On Diff #6155)

Yup updated the description to reflect that.

tools/entity/entvalidate.pl
8 ↗(On Diff #6155)

Yeah that's weird, maybe something for another patch ?

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/353/display/redirect

elexis added inline comments.Apr 7 2018, 12:48 PM
source/tools/entity/checkrefs.pl
194

L180-L193 is copypasta, better put it into a function, so maintenance and extension is half as expensive

Stan updated this revision to Diff 6341.Apr 7 2018, 1:17 PM

Remove broken constants (apparently they do not support being loaded at start)
Add a check for missing referenced parent variant files that will throw errors like:

Missing file 'art/variants/biped/base_driver_charioSRWDHFXJGt_celt.xml' referenced by: 'public/art/actors/units/britons/chariot_javelinist_c_d.xml'

Move the dependencies check to a function to remove redundancy (Which was made me notice we didn't do the above, so thanks)

Vulcan added a comment.Apr 7 2018, 1:20 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/357/display/redirect

Stan added a comment.Jun 22 2018, 9:36 AM

@elexis Any time to accept this ?

Stan added a subscriber: Itms.Jan 1 2019, 7:00 PM

@Itms @elexis Can I commit this ?

Itms added a comment.Jan 2 2019, 1:57 PM

I am Perl illiterate, but I tested the updated script and it works fine. It detected the following issues for me:

Loading maps PMP...
Duplicate terrain name 'medit_city_tile' (from 'art/terrains/road/medit_city_tile.xml' and 'art/terrains/biome-mediterranean/medit_city_tile.xml')
[...]
Loading GUI XML...
Can't stat ../../../binaries/data/mods/public/gui/modio: No such file or directory
 at checkrefs.pl line 61.
Can't stat ../../../binaries/data/mods/public/gui/modmod/help: No such file or directory
 at checkrefs.pl line 61.
Can't stat ../../../binaries/data/mods/public/gui/msgbox: No such file or directory
 at checkrefs.pl line 61.
Can't stat ../../../binaries/data/mods/public/gui/termsdialog: No such file or directory
 at checkrefs.pl line 61.
[...]
Loading terrains...

Missing file 'art/textures/ui/session/portraits/' referenced by: 'public/simulation/templates/special/filter/mirage.xml'

So the only comments I would make are:

  • in the GUI XML section, it would be nice to print which is the file referencing those old folders.
  • this extra newline when parsing the terrains could be removed.

But this is just cosmetics. It is easy to grep the XML to know where the issues are, etc. I think a fix to the detected issues should be committed before the script itself is fixed ?

elexis updated the Trac tickets for this revision.Jan 2 2019, 2:14 PM
elexis added a comment.Jan 2 2019, 2:19 PM

The multiple hardcodings of public aren't too nice. It would be better if it would be agonstic of mods and either run over all mods or only on the mods specified via commandline (ideally). At least the public copies should be abstracted by a variable name, not sure where and how exactly.
I also had a diff at https://trac.wildfiregames.com/ticket/4777#comment:1 for some rmbiome and portrait errors and there was #4560 for special filter templates. Otherwise dunno, would have to check.

Stan added a comment.Jan 2 2019, 4:10 PM

I planned for multimod support after this patch. Because I need it for mods. I believe it's out the scope of this patch tho.

Stan added a comment.Jan 3 2019, 9:55 AM

@elexis I will include your fix at #4777 in this patch do you want me to also include D1440 ?

Stan updated this revision to Diff 7233.Jan 4 2019, 2:36 AM
Stan edited the summary of this revision. (Show Details)

Now with mod support. Added another param to specify a mod to test. The script will then attempt to load its dependencies.

@Itms can you try again ?

@elexis I included your patch from the ticket witch fixed the remaining issue. The last one is that thing about the duplicate texture, but I don't know how to parse pmp files to figure which of the textures is the most used. it's out of the scope of the ticket.

Should I include your other fix for obstructions ?

Vulcan added a comment.Jan 4 2019, 2:37 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/899/

Stan updated this revision to Diff 7234.Jan 4 2019, 2:44 AM
Vulcan added a comment.Jan 4 2019, 2:45 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/900/

elexis added a comment.Jan 4 2019, 1:42 PM

Certainly not D1440, that thing is dubious. Also not sure if the biome diff needs to be included in the same patch, nor sure if my diff was the right implementation, just wanted to point out that it exists and that we had some error messages in the output about that IIRC.

source/tools/entity/Entity.pm
116

(I didn't read the code, but should one mod stand out as the 'main' mod, or should it just iterate through all mods and treat each mod equally?
Or is main_mod the only mod considered and it could become get_mod? (As I recall it checked the XML files of the modmod, no?))

source/tools/entity/creationgraph.pl
6 ↗(On Diff #7234)

Fair enough "public" abstraction for now, but ideally no mod name would exist here at all. But it's probably better to implement that in a separate diff to prevent this patch from becoming too big.

Stan marked 2 inline comments as done.Jan 4 2019, 1:48 PM

The biome implementation worked as expected, so I included it. If you have a better one though feel free to suggest :)

source/tools/entity/Entity.pm
116

Well actually it's the main mod by default for example

terra_magna|public|mod would return terra_magna, but only if the file we are looking for exist in that mod. So it's the highest mod of the stack to be precise.

source/tools/entity/creationgraph.pl
6 ↗(On Diff #7234)

Well since I break that script in that patch and I'm not exactly sure what it does or where it's used I'd rather include it there. Could be made in separate commits though.

Itms added a comment.Jan 4 2019, 10:04 PM

This still works for me, with the same results. I didn't test a mod but the cli flag seems to work for the public mod as advertised.

I have a question regarding the unused files flag: I didn't look at the list too much because I know we have a lot of unused art, but I actually see references to files that are unlikely to be unused (like the persian voices). Is this working correctly for you? I can paste the list the script gives for me.

Stan updated this revision to Diff 7261.EditedJan 5 2019, 3:09 PM
  • Add missing rootactor push in soundgroup (Fix for voices being unused)
  • Add missing rootactor push in terrains
  • Add auras.
  • Enable mapxml when check unused is enabled
  • Fix crash when civ has no emblem.
  • Remove trailing whitespaces.
  • Remove useless line jumps
  • Simplify check in check_unused()

Should be good now. Unused will always be full of false positives until we move all the icons.png loading out of js files and tips are transformed into a parsable format like Json. I will try to nuke the rest in the coming months.

Vulcan added a comment.Jan 5 2019, 3:44 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/917/

Stan added a comment.Feb 24 2019, 5:06 PM

@Itms can I commit this ?

Itms added a comment.Feb 24 2019, 5:31 PM

Just tested it and it still seems to work pretty well ?

Could you add a README in the folder explaining the usage and options? You could mention that the --check-unused has a bunch of false positives in there.

Stan updated this revision to Diff 7501.Feb 24 2019, 8:24 PM
  • Add readme

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1068/display/redirect

Itms requested changes to this revision.Feb 24 2019, 8:55 PM

Some small improvements needed, sorry...

source/tools/entity/readme.md
8 ↗(On Diff #7501)

Are those <a> something that your Markdown editor adds? Semantically, it's terribly ugly to create anchors like that. Anchors should be created automatically with standard titles.

10 ↗(On Diff #7501)

script*

29 ↗(On Diff #7501)

the option descriptions are not consistent (sometimes it's an infinitive, sometimes a 3rd person, and the last one describes the syntax instead of the aim)

30 ↗(On Diff #7501)

Please state that this currently yields false positives.

33 ↗(On Diff #7501)

Please give the default value.

This revision now requires changes to proceed.Feb 24 2019, 8:55 PM
Stan updated this revision to Diff 7502.Feb 24 2019, 9:02 PM
Stan marked 5 inline comments as done.
Stan added inline comments.
source/tools/entity/readme.md
8 ↗(On Diff #7501)

Can't have anchors with 1. In their names. Removed the TOC then.

Itms accepted this revision.Feb 24 2019, 9:25 PM

Looks good to me, just fix the Frankenstein sentence in the readme before committing ?

source/tools/entity/readme.md
28 ↗(On Diff #7502)

this sentence has been rewritten too many times ?

Replace mod by mods in the option syntax.

specify which mods to check. 'mods' should be a list of mods separated by '|'. Default value: 'public|mod'.

This revision is now accepted and ready to land.Feb 24 2019, 9:25 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1069/display/redirect

Stan updated this revision to Diff 7503.Feb 24 2019, 9:38 PM
Stan marked an inline comment as done.
  • Fix frankenstein.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1070/display/redirect

This revision was automatically updated to reflect the committed changes.