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 AppearsOnMapPreview #15731

Merged
merged 2 commits into from Apr 4, 2019
Merged

Conversation

MustaphaTR
Copy link
Member

@MustaphaTR MustaphaTR commented Oct 25, 2018

Adds required logic to fix "#12097 and #13823". Tho i haven't actually updated any map to actually fix the issue.

I put a testcase for stuff that preferably be rendered on RA mod. You can test by opening a map and saving it (seems like it requires a restart so it'll show the new preview).

@SoScared
Copy link
Member

I assume you want to put RenderOnMapPreview on ^Building as well https://github.com/OpenRA/OpenRA/pull/15731/files#diff-5b0780203fcfc47fcb88e0b8d6d306b9R638

pchote
pchote previously requested changes Oct 25, 2018
OpenRA.Game/Map/Map.cs Outdated Show resolved Hide resolved
@pchote
Copy link
Member

pchote commented Oct 25, 2018

This is a high impact feature, so adding it to the milestone.

@pchote pchote force-pushed the render-on-map-preview branch 2 times, most recently from 0bcd9a9 to a07c596 Compare November 4, 2018 15:05
@pchote
Copy link
Member

pchote commented Nov 4, 2018

I have pushed over this with some changes to streamline the feature plumbing. 👍 to that part of the PR now.

Two things left to go:

  • Add AppearsOnMapPreview to the other default mods.
  • Implement an update rule and update all the default maps - we will need to add new plumbing to the update rule infrastructure, so might be best if I handle this part.

@pchote
Copy link
Member

pchote commented Nov 24, 2018

Ping @MustaphaTR

@pchote pchote modified the milestones: Next Release, Next + 1 Nov 25, 2018
@pchote
Copy link
Member

pchote commented Nov 25, 2018

I would have really liked this for the current release, but the amount of work here (including regenerating all maps and getting them on the resource center) is too risky for the short window that remains for prepping a christmas playtest. Bumping to Next + 1.

@MustaphaTR
Copy link
Member Author

Implement an update rule and update all the default maps - we will need to add new plumbing to the update rule infrastructure, so might be best if I handle this part.

I think last we agreed that you were going to do this and i would do the updating the shipped maps.

@pchote
Copy link
Member

pchote commented Mar 31, 2019

I've been waiting for the other mods to be updated, which is checked in the list above but not actually done!?

@MustaphaTR
Copy link
Member Author

Ok, i thought i've done that, but it appears that i have actually only appied it to RA. Will rebase this first and do so.

Allows preplaced actors to be rendered on map preview when saving the
map.

Also removes requirements for ResourceLayer in OpenRA.Game and moves it
to OpenRA.Mods.Common.
@MustaphaTR
Copy link
Member Author

Updated.

@MustaphaTR
Copy link
Member Author

Updated.

mods/cnc/rules/defaults.yaml Show resolved Hide resolved
mods/cnc/rules/trees.yaml Outdated Show resolved Hide resolved
mods/d2k/rules/structures.yaml Show resolved Hide resolved
mods/ts/rules/defaults.yaml Show resolved Hide resolved
@pchote
Copy link
Member

pchote commented Mar 31, 2019

I'll file a followup PR that makes some tweaks to the map.yaml formatting, adds a new --refresh-map utility command, and ports over the default mod maps after this has been merged.

To Walls, (Tib)Trees, Ore/Gem Mines and Tech Structures.
Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that AppearsOnMapPreview is now declared on all actors that use RadarColorFromTerrain plus tech structures and walls. The rest of the code still LGTM.

@reaperrr reaperrr changed the title Add RenderOnMapPreview Add AppearsOnMapPreview Apr 4, 2019
@reaperrr reaperrr merged commit 9ec8d25 into OpenRA:bleed Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants