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

Implementing Doe's new banners and signs. #4582

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

MistakeNot4892
Copy link
Contributor

@MistakeNot4892 MistakeNot4892 commented Nov 24, 2024

Random debug types to show the concepts:
image
image

Icons by @PlayerDeer.

@MistakeNot4892 MistakeNot4892 added the ready for review This PR is ready for review and merge. label Nov 24, 2024
@MistakeNot4892 MistakeNot4892 changed the title Implementing Doe's new banners. Implementing Doe's new banners and signs. Nov 26, 2024
out-of-phaze
out-of-phaze previously approved these changes Nov 29, 2024
Copy link
Member

@out-of-phaze out-of-phaze left a comment

Choose a reason for hiding this comment

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

presumably at some point modpacks should be able to add additional banner icons, e.g. corporate logos as decals in the corporate modpack, holy symbols as decals in the mundane/modern earth modpack, etc.

would also be nice to, in the future, have it list decals on examine, including their color if supported. "This is a <font color="white">banner</font> with a <font color="red">sword</span> embroidered on it.", `"This is a walnut sign with a beer mug embroidered on it." etc.

for now this looks good, might be worth adding a unit test in the future that checks if the various states exist for every banner type's base_icon_state, like banner-hanging etc.

I.color = banner.color
add_overlay(I)

if(banner.trim_color)
Copy link
Member

Choose a reason for hiding this comment

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

kind of feel like the decals should be added to the banner state first, with BLEND_INSET_OVERLAY, and then we apply the trim on top to keep the layering sane. that way you could have user-added decals like stripes/crosses/etc that aren't mutually exclusive with other decals and work regardless of banner shape. this probably works fine as-is i just wanted to point out that inset overlay blend mode could be useful here now or in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@MistakeNot4892
Copy link
Contributor Author

presumably at some point modpacks should be able to add additional banner icons, e.g. corporate logos as decals in the corporate modpack, holy symbols as decals in the mundane/modern earth modpack, etc.

Symbols are now decls, so this is done.

would also be nice to, in the future, have it list decals on examine, including their color if supported. "This is a <font color="white">banner</font> with a <font color="red">sword</span> embroidered on it.", `"This is a walnut sign with a beer mug embroidered on it." etc.

Simple version of this is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR is ready for review and merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants