8000 Add fluid line depth overlays to fluids by FlameArrow57 · Pull Request #23213 · goonstation/goonstation · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add fluid line depth overlays to fluids #23213

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Add fluid line depth overlays to fluids #23213

wants to merge 13 commits into from

Conversation

FlameArrow57
Copy link
Contributor
@FlameArrow57 FlameArrow57 commented Apr 29, 2025 •
edited
Loading

[GAME OBJECTS][FEATURE]

About the PR

Title, fluid lines are shown for deep fluids

image

image

Sprites partially worked on by Skeletonman0

Why's this needed?

Would be cool, shows more depth to fluids

@keywordlabeler keywordlabeler bot added A-Game-Objects The point of this PR is to deal with a specific game object C-Feature A new feature or enhancements to existing features labels Apr 29, 2025
@boring-cyborg boring-cyborg bot added the C-Sprites Automatically applied on any .dmi or icons folder change label Apr 29, 2025
@github-actions github-actions bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 29, 2025
@FlameArrow57 FlameArrow57 marked this pull request as ready for review April 29, 2025 16:37
@itsloley
Copy link
Contributor

i feel like the lines might be too colorful and/or bright? kinda makes it look more like a "magical mist/fog" instead of liquid, might be just me though!!

@FlameArrow57
Copy link
Contributor Author

i feel like the lines might be too colorful and/or bright? kinda makes it look more like a "magical mist/fog" instead of liquid, might be just me though!!

What about now?

TobleroneSwordfish
TobleroneSwordfish reviewed May 7, 2025
View reviewed changes
Copy link
Contributor
@TobleroneSwordfish TobleroneSwordfish left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Is it feasible to bake these into the icon states instead? That's a huge amount of extra overlays for big fluid pools which may impact client performance.

@FlameArrow57
Copy link
Contributor Author

Is it feasible to bake these into the icon states instead? That's a huge amount of extra overlays for big fluid pools which may impact client performance.

Possibly, the fluid objects will need to have their layers changed so that the fluid lines appear over objects if so

@FlameArrow57 FlameArrow57 marked this pull request as draft May 8, 2025 03:39
@FlameArrow57 FlameArrow57 marked this pull request as ready for review May 8, 2025 13:25
TobleroneSwordfish
TobleroneSwordfish requested changes May 18, 2025
View reviewed changes
Copy link
Contributor
@TobleroneSwordfish TobleroneSwordfish left a comment

Choose a reason for hiding this comment

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

Oh uh, that's way worse. Objects are heavier than images! I meant is it possible to just put the lines on the existing fluid icon states rather than making them separate at all.

@FlameArrow57
Copy link
Contributor Author

Oh uh, that's way worse. Objects are heavier than images! I meant is it possible to just put the lines on the existing fluid icon states rather than making them separate at all.

I don't think so, it makes them very hard to see for more transparent fluids like water

@github-actions github-actions bot added the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label May 20, 2025
@TobleroneSwordfish
Copy link
Contributor

Fluids already trash client performance pretty badly, I don't think adding another O(n^2) number of appearances per fluid puddle is worth it. Surely the lines being more transparent on more transparent fluids would be intentional anyway?

@github-actions GitHub Actions
Copy link
Contributor
github-actions bot commented Jun 5, 2025

This PR has been inactive for two weeks, and has been automatically marked as stale. This means it is at risk of being auto closed in another week. Please address any outstanding review items and ensure your PR is finished. If you are auto-staled anyway, ask developers if your PR will be merged. Once you have done any of the previous actions then you should request a developer remove the stale label on your PR, to reset the stale timer. If you feel no developer will respond in that time, you may wish to close this PR youself, while you seek developer comment, as you will then be able to reopen the PR yourself.

@github-actions github-actions bot added the S-Stale An inactive PR that has had no updates in the past two weeks label Jun 5, 2025
@github-actions github-actions bot removed the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Jun 12, 2025
@github-actions github-actions bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 14, 2025
@github-actions github-actions bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 14, 2025
@FlameArrow57 FlameArrow57 marked this pull request as draft June 14, 2025 00:50
@FlameArrow57 FlameArrow57 removed the request for review from TobleroneSwordfish June 14, 2025 00:51
@github-actions github-actions bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 14, 2025
@FlameArrow57 FlameArrow57 marked this pull request as ready for review June 14, 2025 01:16
@FlameArrow57 FlameArrow57 removed the S-Stale An inactive PR that has had no updates in the past two weeks label Jun 14, 2025
pgmzeta
pgmzeta approved these changes Jun 14, 2025
View reviewed changes
Copy link
Contributor
@pgmzeta pgmzeta left a comment

Choose a reason for hiding this comment

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

there's a little bit of 'pop-in' if the fluid takes a bit to settle, such as a large volume in a small room escapes into a larger area. I don't think that's a deal-breaker, though.

cannot speak as to performance change.

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

@pgmzeta pgmzeta pgmzeta approved these changes

@TobleroneSwordfish TobleroneSwordfish Awaiting requested review from TobleroneSwordfish

Assignees
No one assigned
Labels
A-Game-Objects The point of this PR is to deal with a specific game object C-Feature A new feature or enhancements to existing features C-Sprites Automatically applied on any .dmi or icons folder change size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@FlameArrow57 @itsloley @TobleroneSwordfish @pgmzeta
0