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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify and polish D2k effects/palettes. #21210

Merged
merged 9 commits into from Nov 15, 2023

Conversation

pchote
Copy link
Member

@pchote pchote commented Nov 13, 2023

This PR does some initial housekeeping around D2k palettes in preparation for using the R16 sprites (aka D2kHDR 馃寛).

I compared most of our effects against the "Dune 2000 Map and Mission Editor" tool which caught some discrepencies that I have now also fixed.

Screenshot 2023-11-13 at 20 16 30

The cloak palette will be handled in its own PR.

The fog now looks slightly different, i'm hoping this won't be too controversial.

@penev92
Copy link
Member

penev92 commented Nov 14, 2023

Code changes look fine, but this needs to be scrutinized in game. 馃

@Porenutak
Copy link
Contributor

New Fog of War change whole color tone instead of just make scene darker or foggy. Its awkward and not very eye pleasing.

Screenshot (235)

Rest is OK.

@PunkPun
Copy link
Member

PunkPun commented Nov 14, 2023

I agree. The new fog feels like it has a high contrast filter applied. The east and north facing cliffs look quite ugly

Old New

@PunkPun
Copy link
Member

PunkPun commented Nov 14, 2023

Screenshot 2023-11-14 at 19 31 48

@pchote
Copy link
Member Author

pchote commented Nov 14, 2023

Updated to restore the previous fog visuals, and added a new commit to use the correct trike icon (4305 is a duplicate of 4277 in DATA.R8, but garbage in DATA.R16).

@Porenutak
Copy link
Contributor

LGTM

PunkPun
PunkPun previously approved these changes Nov 15, 2023
Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

Actually shouldn't there be an update rule for the trait removal?

@pchote
Copy link
Member Author

pchote commented Nov 15, 2023

They are d2k-specific traits in the Mods.D2k.dll, so IMO no (and this would cause problems with visibility since UpgradePath can't see into Mods.D2k.dll).

@PunkPun
Copy link
Member

PunkPun commented Nov 15, 2023

But in update we only deal in strings. We don't use actual classes that could have dependency issues unless it's basic structs such as CPos.

And we do write update rules for Mods.Cnc.dll

@pchote
Copy link
Member Author

pchote commented Nov 15, 2023

Added update rules.

@PunkPun
Copy link
Member

PunkPun commented Nov 15, 2023

needs a rebase

@pchote
Copy link
Member Author

pchote commented Nov 15, 2023

Rebased.

Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

LGTM

@PunkPun PunkPun merged commit 60cbf79 into OpenRA:bleed Nov 15, 2023
3 checks passed
@PunkPun
Copy link
Member

PunkPun commented Nov 15, 2023

Changelog

@pchote pchote deleted the d2k-effect-polish branch November 15, 2023 20:57
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