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

[Squash] Add paramtype2 colorflowingliquid #15180

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

DragonWrangler1
Copy link
Contributor

@DragonWrangler1 DragonWrangler1 commented Sep 18, 2024

Closes #12897.

this is an adoption of #12907

  • colorflowingliquid has been added.
  • The upper four bits of flowing liquid nodes are not cleared by liquid transformation.
  • post_effect_use_node_color has been added, which lets the post effect color vary depending on a node's param2 color.

To do

This PR is Ready for Review.

  • Are nodes colored correctly when using post_effect_use_node_color? It was copied from the procedure of the [multiply texture modifier.

How to test

  1. Try the new liquid whose source node is testnodes:colorflowingliquid_source. Try changing the color of some flowing liquid using the param2 tool. Make sure the post effect color varies with the palette index. (has up to 16 colors)
  2. Test the liquid color and post effect color on an old client.
  3. Try connecting to an old server with a new client.
  4. Make sure liquid flow still works.

@sfence sfence added Feature ✨ PRs that add or enhance a feature @ Server / Client / Env. labels Sep 18, 2024
@Zughy Zughy changed the title [ADOPTION] Colorflowingliquid Add paramtype2 colorflowingliquid Sep 18, 2024
@Zughy Zughy added the Concept approved Approved by a core dev: PRs welcomed! label Sep 18, 2024
@grorp
Copy link
Member

grorp commented Sep 18, 2024

Attribution to the original author needs to be preserved in Git history.

@DragonWrangler1
Copy link
Contributor Author

DragonWrangler1 commented Sep 18, 2024

@grorp And how do I do that ? I'm working on a totally different branch, in a totally different repo, and a totally different version.

@rollerozxa
Copy link
Member

@grorp And how do I do that ? I'm working on a totally different branch, in a totally different repo, and a totally different version.

Add them as a co-author in commits. When a PR is squashed any commit should work to make them be attributed in the squashed merged commit.

https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

@DragonWrangler1
Copy link
Contributor Author

@rollerozxa Thanks.

DragonWrangler1 and others added 2 commits September 18, 2024 14:47
Co-authored-by: TurkeyMcMac <[email protected]>
@DragonWrangler1 DragonWrangler1 changed the title Add paramtype2 colorflowingliquid [Squash] Add paramtype2 colorflowingliquid Sep 18, 2024
@corarona
Copy link
Contributor

corarona commented Sep 19, 2024

you can also put any author you want to using git commit --author="Author <[email protected]>"

@Wbjitscool
Copy link

while liquid is being worked on I was just wondering if it is possible to adjust the liquid node heights too like in a setting or something?

@DragonWrangler1
Copy link
Contributor Author

DragonWrangler1 commented Sep 25, 2024

while liquid is being worked on I was just wondering if it is possible to adjust the liquid node heights too like in a setting or something?

It is possible, but I won’t be doing that in this pr.

@Wbjitscool
Copy link

okay

@Zughy Zughy added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Dec 12, 2024
Copy link
Contributor

@appgurueu appgurueu left a comment

Choose a reason for hiding this comment

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

Are nodes colored correctly when using post_effect_use_node_color? It was copied from the procedure of the [multiply texture modifier.

Seems sensible to me, the shaders just do a RGB multiplication for hardware coloring as well.


A question: What would / should happen if a node is "flowed into" by two neighboring flowing liquids of the same type but different colors?

@DragonWrangler1
Copy link
Contributor Author

Apologies for the delay. I’ll resume work on this shortly. Had some irl work to do, before spending too much time on this.

@DragonWrangler1
Copy link
Contributor Author

Are nodes colored correctly when using post_effect_use_node_color? It was copied from the procedure of the [multiply texture modifier.

Seems sensible to me, the shaders just do a RGB multiplication for hardware coloring as well.

A question: What would / should happen if a node is "flowed into" by two neighboring flowing liquids of the same type but different colors?

Should randomly pick one of the colors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Action / change needed Code still needs changes (PR) / more information requested (Issues) Concept approved Approved by a core dev: PRs welcomed! Feature ✨ PRs that add or enhance a feature @ Server / Client / Env.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flowing Liquids Should Not Use Param2 For Flow Direction
8 participants