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

Implement new mod updater framework. #14964

Merged
merged 1 commit into from Mar 23, 2018

Conversation

Projects
None yet
3 participants
@pchote
Copy link
Member

pchote commented Mar 21, 2018

This PR implements the plumbing promised in #14600 and ports over the rules from release-20180218 to current bleed. I would like to think that we can extend this back to release-20171014 in a future PR to provide continuity for modders on the Mod SDK (I have a separate commit that makes a start on this). Depends on #14948.

The key points to know about the new system are:

  • All changes to the mod rules are expected to define an update rule, even if it only lists the changes that a modder needs to make manually.
  • The default commands for the updater list information about the changes; an explicit --apply command must be given before any changes are made. This makes the updater our primary documentation for compatibility changes.
  • Each update "rule" is considered independently, and can be applied individually (e.g. when working on bleed), or as part of an "update path" from a labelled tag to the current HEAD.
  • If a rule throws an exception then its changes will be reverted and any other updates in the path will be applied.

I expect there will be some bugs to shake out before we merge this, so testing by third party mods would be appreciated. I am deliberately not explaining how to use this (except to note that it has been renamed to --update-mod / --update-map), because one of the main goals of this new system is to be self-explanatory and relatively intuitive.

@pchote pchote added this to the Next release milestone Mar 21, 2018

@@ -0,0 +1,114 @@
#region Copyright & License Information
/*
* Copyright 2007-2017 The OpenRA Developers (see AUTHORS)

This comment has been minimized.

@reaperrr

reaperrr Mar 21, 2018

Contributor

2018 (applies to all new files).

@pchote pchote force-pushed the pchote:upgrade-rules branch 2 times, most recently from b9eb6ef to 5ddb324 Mar 21, 2018

+ "You may wish to explicitly define `" + replacement + "` at the following locations\n"
+ "if the sound has not already been inherited from a parent definition.\n";

foreach (var n in nodes)

This comment has been minimized.

@pchote

pchote Mar 21, 2018

Author Member

I should split out a helper for aggregating lines into sub-lists. This is going to be done a lot.

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Mar 21, 2018

#14948 was merged.

@abcdefg30
Copy link
Member

abcdefg30 left a comment

Looks good to me otherwise from a first look. 👍
The manual changes section can become a bit spammy, but I guess that is to be expected.

}

if (args.Contains("--apply"))
{

This comment has been minimized.

@abcdefg30

abcdefg30 Mar 21, 2018

Member

Braces around a single-line statement.

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Mar 21, 2018

Updated and rebased.

Yes, this is designed to be spammy: if the rules don't explain everything that a modder needs to do to restore compatibility then the rules aren't written well enough.

@pchote pchote force-pushed the pchote:upgrade-rules branch from 4f779e9 to b2803eb Mar 21, 2018

@pchote pchote force-pushed the pchote:upgrade-rules branch from b2803eb to b351b6a Mar 21, 2018

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Mar 21, 2018

Updated again. I removed the widget updating plumbing because I want to change that, and it doesn't have any uses in the current PR. We can add that back when porting over the older rules.

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Mar 23, 2018

Tested locally with @KOYK's TDA mod, didn't run into any issues, all commands and source types seem to work fine 👍

@reaperrr reaperrr merged commit ea68f1a into OpenRA:bleed Mar 23, 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 referenced this pull request Apr 29, 2018

Closed

engineVersion pseudo-dates #9899

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.