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

Separated resource rendering into another trait #16710

Merged
merged 1 commit into from Jan 14, 2020

Conversation

@teinarss
Copy link
Contributor

teinarss commented Jun 18, 2019

Resurrection of #13902

Added the ResourceRenderer to the player traits folder cuz of the comment from pchote in the previous PR

mods/ts/rules/world.yaml Outdated Show resolved Hide resolved
@teinarss teinarss force-pushed the teinarss:reslayer_ref branch from 6a1595b to 9f385ea Jun 28, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Jun 28, 2019

Updated: Fixed veins

Copy link
Member

abcdefg30 left a comment

Code looks otherwise good on a first pass.

This will want an update rule.

OpenRA.Mods.D2k/Traits/World/D2kResourceRenderer.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/Player/ResourceRenderer.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/Player/ResourceRenderer.cs Outdated Show resolved Hide resolved
@teinarss teinarss force-pushed the teinarss:reslayer_ref branch from 9f385ea to 874de34 Oct 13, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Oct 13, 2019

Updated

@teinarss teinarss force-pushed the teinarss:reslayer_ref branch from 874de34 to 200709a Oct 13, 2019
Copy link
Member

abcdefg30 left a comment

This doesn't look correct. (?)
grafik

@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Dec 17, 2019

This doesn't look correct. (?)
grafik

What map is this and where on the map?

@teinarss teinarss force-pushed the teinarss:reslayer_ref branch from 200709a to cbbd1ec Dec 17, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Dec 17, 2019

Updated

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Dec 17, 2019

What map is this and where on the map?

Battle for Dune in the upper right.

@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Dec 19, 2019

What map is this and where on the map?

Battle for Dune in the upper right.

Need more info, still cant reproduce it. I did rebase it on bleed. Is in still present for you?

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Dec 19, 2019

Testcase is simply revealing what another player harvested.

@teinarss teinarss force-pushed the teinarss:reslayer_ref branch from cbbd1ec to d3a213a Dec 26, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Dec 26, 2019

Updated

@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Dec 31, 2019

Made the following improvements too:

  • Removed the unnecessary Sprite from CellContents. It was only needed because spriteLayers was updated from CellEntryChanged. Updating the spriteLayers directly now instead of going through the celllayer.
  • Removed some unnecessary access to the CellLayers. Now these values are passed to the needed methods.
  • Removed one unnecessary allocation.
Copy link
Member

pchote left a comment

Looks mostly good to me. Spotted one regression and have a couple of questions/suggestions.

OpenRA.Mods.Common/Traits/World/ResourceLayer.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/Harvester.cs Outdated Show resolved Hide resolved
OpenRA.Mods.D2k/Traits/World/D2kResourceRenderer.cs Outdated Show resolved Hide resolved
OpenRA.Mods.D2k/Traits/World/D2kResourceRenderer.cs Outdated Show resolved Hide resolved
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Jan 12, 2020

Updated

@pchote pchote added the PR: Needs +2 label Jan 12, 2020
@teinarss teinarss force-pushed the teinarss:reslayer_ref branch from c20526d to 065e038 Jan 12, 2020
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Jan 12, 2020

Upated

@abcdefg30 abcdefg30 merged commit f0b69f8 into OpenRA:bleed Jan 14, 2020
2 checks passed
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 Jan 14, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.