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

Add lint tests for unknown trait and weapon fields #15288

Merged
merged 4 commits into from Jun 30, 2018

Conversation

Projects
None yet
4 participants
@pchote
Copy link
Member

pchote commented Jun 22, 2018

Fixes #15201, with several key improvements over #15264:

  • Doesn't hook into the internal game logic.
  • Lists all errors in a single pass.
  • Displays filenames and line numbers instead of just actor/trait name.

@pchote pchote added this to the Next release milestone Jun 22, 2018

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Jun 22, 2018

Testcase: rebase this PR onto release-20180307 and notice that all the recently fixed errors in the default mods are reported.

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Jun 22, 2018

Nope, looks like this is RIP too.

Definitions like the following break the approach here because we don't know what type of warhead it corresponds to:

BarrelExplode:
	Warhead@1Dam:
		ValidTargets: Ground, Prisoner

@pchote pchote closed this Jun 22, 2018

@pchote pchote reopened this Jun 25, 2018

@pchote pchote force-pushed the pchote:trait-lint branch from ff869eb to 0a585b8 Jun 25, 2018

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Jun 25, 2018

Added a workaround for the case above:

OpenRA.Utility(1,1): Warning: weapons.yaml:10 does not define a warhead type. Skipping unknown field check.

and squashed the warnings in the default missions.

@GraionDilach

This comment has been minimized.

Copy link
Contributor

GraionDilach commented Jun 26, 2018

I'll be honest - I thought respecifying the warhead types in definitions were good practice.

@GraionDilach
Copy link
Contributor

GraionDilach left a comment

👍 though.

@ABrandau

This comment has been minimized.

Copy link
Contributor

ABrandau commented Jun 27, 2018

Testing mod: Red Alert

OpenRA.Utility(1,1): Error: ra|rules/defaults.yaml:77 refers to a trait field `FirepowerMultiplierDoesntContainThis` that does not exist on `FirepowerMultiplier`.
OpenRA.Utility(1,1): Error: ra|rules/defaults.yaml:78 refers to a trait field `FakeTrait` that does not exist on `FirepowerMultiplier`.
OpenRA.Utility(1,1): Error: ra|weapons/smallcaliber.yaml:6 refers to a weapon field `InvalidWeaponProperty` that does not exist.
OpenRA.Utility(1,1): Error: ra|weapons/smallcaliber.yaml:10 refers to a projectile field `InvalidProjectileProperty` that does not exist on `Bullet`.
OpenRA.Utility(1,1): Error: ra|weapons/smallcaliber.yaml:14 refers to a warhead field `WrongDamageWarheadProperty` that does not exist on `SpreadDamage`.

Only detail is that it doesnt look at the sequences, not like that matters much anyway, I didnt made it look at Chrome, metrics or other details like that either (might be important for some).

Otherwise, it works like a charm. 👍

@pchote pchote merged commit 39a3bc4 into OpenRA:bleed Jun 30, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@pchote pchote deleted the pchote:trait-lint branch Jul 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.