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 more node utility extensions for update rules #15219

Merged
merged 4 commits into from Jun 15, 2018

Conversation

Projects
None yet
3 participants
@reaperrr
Copy link
Contributor

reaperrr commented Jun 7, 2018

Adds MoveNode, MoveAndRenameNode and RenameChildrenMatching extensions.

Only updated 4 rules to serve as examples, but #15010 has some prime candidates that could benefit a lot from this (in particular, ReworkCheckboxes and SplitGateFromBuilding).

For the record, I tested all new extensions and updated rules.

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Jun 7, 2018

Note: Some rules in #15010 could be shortened a lot and as result become a bit easier to review if this PR could get reviewed and merged quickly.

@reaperrr reaperrr force-pushed the reaperrr:update-extensions1 branch from f52981b to bcdd5f0 Jun 7, 2018


intensity.RenameKeyPreservingSuffix("Duration");
}
sod.RenameLastChildMatching("Intensity", "Duration");

This comment has been minimized.

@pchote

pchote Jun 7, 2018

Member

When renaming nodes we really should be doing everything, not just the last one. Otherwise the update will leave you in a broken state if you're renaming a trait node and an actor has multiple.

fromNode.Value.Nodes.Remove(node);
}

public static void MoveAndRenameNode(this MiniYamlNode node, MiniYamlNode fromNode, MiniYamlNode toNode, string newKey)

This comment has been minimized.

@pchote

pchote Jun 7, 2018

Member

Can we keep the PreservingSuffix here for consistency?

This comment has been minimized.

@reaperrr

reaperrr Jun 7, 2018

Author Contributor

Made preserving suffix optional instead (defaulting to "true", though).

@reaperrr reaperrr force-pushed the reaperrr:update-extensions1 branch from bcdd5f0 to c058f89 Jun 7, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Jun 7, 2018

Updated.

@reaperrr reaperrr force-pushed the reaperrr:update-extensions1 branch from c058f89 to 19ae333 Jun 7, 2018

uneios.RenameKeyPreservingSuffix("-SpawnActorsOnSell");
public override IEnumerable<string> UpdateActorNode(ModData modData, MiniYamlNode actorNode)
{
actorNode.RenameChildrenMatching("-EmitInfantryOnSell", "-SpawnActorsOnSell");

This comment has been minimized.

@pchote

pchote Jun 9, 2018

Member

We are generally really bad about (not) renaming removals in most cases. How about we add a bool includeRemovals=true parameter to the *Rename* helpers so that we don't need to worry about it?

}
foreach (var eios in actorNode.ChildrenMatching("EmitInfantryOnSell"))
{
eios.RenameKeyPreservingSuffix("SpawnActorsOnSell");

This comment has been minimized.

@pchote

pchote Jun 9, 2018

Member

Can we please change RenameKeyPreservingSuffix to RenameKey(..., bool preserveSuffix=true) for consistency?


node.MoveNode(fromNode, toNode);
}

/// <summary>Removes children with keys equal to [match] or [match]@[arbitrary suffix]</summary>
public static int RemoveNodes(this MiniYamlNode node, string match)

This comment has been minimized.

@pchote

pchote Jun 9, 2018

Member

We probably also want a bool matchSuffix=true parameter on these too for consistency. Thankfully none of our the rules will need to change to go with this.

@reaperrr reaperrr force-pushed the reaperrr:update-extensions1 branch from 19ae333 to d477737 Jun 14, 2018

reaperrr added some commits May 21, 2018

Refactor some update util extensions
Using arguments instead of separate overloads, plus better support for
automatically handling trait/property removals ('-' prefix).
Add some more useful extensions to UpdateUtils
In many situations, these will save up to 3-4 lines, because quite often this will be enough to avoid curly brackets, "if"s or one-by-on renamings.

@reaperrr reaperrr force-pushed the reaperrr:update-extensions1 branch from d477737 to 1db2565 Jun 14, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Jun 14, 2018

Updated.

Reviewing commit by commit recommended, gh messed up the order a little, 1db2565 is actually the last commit.

@abcdefg30 abcdefg30 merged commit 3a60dcd into OpenRA:bleed Jun 15, 2018

2 checks passed

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

This comment has been minimized.

Copy link
Member

abcdefg30 commented Jun 15, 2018

@reaperrr reaperrr deleted the reaperrr:update-extensions1 branch Aug 5, 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.