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

Rename EditorTilesetFilter to MapEditorData #15630

Merged
merged 1 commit into from Sep 22, 2018

Conversation

Projects
None yet
5 participants
@Ectras
Copy link
Contributor

Ectras commented Sep 20, 2018

Closes #13857.
Renamed the EditorTilesetFilter class to MapEditorData (and EditorTilesetFilterInfo to MapEditorDataInfo).
Created an UpdateRule (RenameEditorTilesetFilter.cs). Updated all four mods with it.

@Ectras

This comment has been minimized.

Copy link
Contributor

Ectras commented Sep 20, 2018

Moved the UpdateRule to the Rules folder, filtered out the .ormap files

<Copy
SourceFiles="$(TargetPath).mdb"
DestinationFolder="$(SolutionDir)mods/common/"
Condition="Exists('$(TargetPath).mdb')"/>

This comment has been minimized.

@Mailaender

Mailaender Sep 20, 2018

Member

Please don't commit these changes.

@Ectras

This comment has been minimized.

Copy link
Contributor

Ectras commented Sep 20, 2018

I'm really sorry for that. I committed the original file now (with just the necessary changes). Hope that's ok.

@pchote
Copy link
Member

pchote left a comment

Code changes look reasonable aside from two minor nits. I'm not able to test this ingame.


public override IEnumerable<string> UpdateActorNode(ModData modData, MiniYamlNode actorNode)
{
foreach (var spawner in actorNode.ChildrenMatching("EditorTilesetFilterInfo"))

This comment has been minimized.

@pchote

pchote Sep 20, 2018

Member

The Info classes are an internal implementation detail - they aren't exposed to the yaml, so you don't need to worry about them here :)

foreach (var spawner in actorNode.ChildrenMatching("EditorTilesetFilterInfo"))
spawner.RenameKey("MapEditorDataInfo");

foreach (var spawner in actorNode.ChildrenMatching("EditorTilesetFilter"))

This comment has been minimized.

@pchote

pchote Sep 20, 2018

Member

Please rename this variable to editor or data or something similar (copy/paste leftover from WormSpawner).

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Sep 20, 2018

We'll also want you to squash the commits together once all the fixups are sorted

@Ectras

This comment has been minimized.

Copy link
Contributor

Ectras commented Sep 20, 2018

I feel a bit like a fool right now. Well, once again, really sorry. Fixed the two mistakes and squashed everything into one commit. Hope I didn't miss anything this time.

@teinarss
Copy link
Contributor

teinarss left a comment

I did some testing:

  • RenameEditorTilesetFilter works as intended
  • No problems found in ActorSelectorLogic

@abcdefg30 abcdefg30 merged commit d9946f6 into OpenRA:bleed Sep 22, 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 Sep 22, 2018

@Ectras Ectras deleted the Ectras:renameEditorTilesetFilter branch Sep 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment