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

Changes to CE's suits #29850

Merged
merged 14 commits into from
Oct 12, 2024
Merged

Conversation

KingFroozy
Copy link
Contributor

@KingFroozy KingFroozy commented Jul 9, 2024

About the PR

Fixed CE's jumpsuit being noisy with colors, added a proper icon and in-hands, also remade the jumpskirt sprites for consistency. Removed random blank sprites in jumpsuit folder and meta-file. Resprited CE's turtleneck for so said consistency.

Why / Balance

Sprite & colour consistency.

Technical details

None.

Media

chief_engineeer

  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

none.
Changelog

@github-actions github-actions bot added No C# For things that don't need code. Changes: Sprites Should be reviewed or fixed by people who are knowledgeable with spriting or visual design. labels Jul 9, 2024
Copy link
Contributor

github-actions bot commented Jul 9, 2024

RSI Diff Bot; head commit 60c8398 merging into fc1c709
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/Clothing/Uniforms/Jumpskirt/ce.rsi

State Old New Status
equipped-INNERCLOTHING-monkey Modified
equipped-INNERCLOTHING Modified
icon Modified
inhand-left Modified
inhand-right Modified

Resources/Textures/Clothing/Uniforms/Jumpskirt/ce_turtle.rsi

State Old New Status
equipped-INNERCLOTHING-monkey Modified
equipped-INNERCLOTHING Modified
icon Modified
inhand-left Modified
inhand-right Modified

Resources/Textures/Clothing/Uniforms/Jumpsuit/ce.rsi

State Old New Status
equipped-INNERCLOTHING-monkey Modified
equipped-INNERCLOTHING Modified
icon Modified
inhand-left Modified
inhand-right Modified
s-equipped-INNERCLOTHING Removed
s Removed

Resources/Textures/Clothing/Uniforms/Jumpsuit/ce_turtle.rsi

State Old New Status
equipped-INNERCLOTHING-monkey Modified
equipped-INNERCLOTHING Modified
icon Modified
inhand-left Modified
inhand-right Modified

Edit: diff updated after 60c8398

@KingFroozy
Copy link
Contributor Author

Ignore the test fail probably, i didn't see any relation to the PR.

@deepdarkdepths
Copy link
Contributor

Test fail is TryStopNukeOpsFromConstantlyFailing. As usual

Copy link
Member

@EmoGarbage404 EmoGarbage404 left a comment

Choose a reason for hiding this comment

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

A few basic thoughts:

I like the more saturated yellow but the contrast is quite a bit higher for things like the detailing on the turtleneck. That should be fixed.

The darker color on the pants is cool but the posted screenshots have significantly simplified and reduced shading compared to the original. The original design of the pants and all the shading is fine: the only thing i would change is the color. It also has the same high-contrast issues as the top part of the outfit, especially with the darker colors on the outlines. It's especially noticeable on the skirts (To which all of this equally applies)

I do like the new vest and i think the reduction in visual noise helps it a lot. However, not having an outline on both sides of the straps gives it some contrast issues with the yellow suit below it. I think those should be restored.

Additionally, the back of the vest has a section with very weird shading:
image

i feel like it should just continue the pattern on the rest of it and have some light shading on the lowest part.

As a final note, please submit screenshots of the art in-game. Seeing how it looks in a more natural environment is preferable to just screenshots from an editor.

@EmoGarbage404 EmoGarbage404 added the Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. label Jul 17, 2024
@KingFroozy
Copy link
Contributor Author

A few basic thoughts:

I like the more saturated yellow but the contrast is quite a bit higher for things like the detailing on the turtleneck. That should be fixed.

The darker color on the pants is cool but the posted screenshots have significantly simplified and reduced shading compared to the original. The original design of the pants and all the shading is fine: the only thing i would change is the color. It also has the same high-contrast issues as the top part of the outfit, especially with the darker colors on the outlines. It's especially noticeable on the skirts (To which all of this equally applies)

I do like the new vest and i think the reduction in visual noise helps it a lot. However, not having an outline on both sides of the straps gives it some contrast issues with the yellow suit below it. I think those should be restored.

Additionally, the back of the vest has a section with very weird shading: image

i feel like it should just continue the pattern on the rest of it and have some light shading on the lowest part.

As a final note, please submit screenshots of the art in-game. Seeing how it looks in a more natural environment is preferable to just screenshots from an editor.

I did just copy the original colors of the jumpsuit to turtleneck for above said consistency, but sure.
I'll chalk more about it in discord channel so cya

@KingFroozy
Copy link
Contributor Author

Made some requested changes, though colors are left untouched, as they're the same as the normal jumpsuit, so if there are still complaints about them, I'd like to leave it for a different PR.

Chief
Chiefy
Turtle
Turtly

Sorrs for being so late, my butt is too lazy and shy to speak up.

@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. and removed Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. labels Aug 23, 2024
Copy link
Member

@UbaserB UbaserB left a comment

Choose a reason for hiding this comment

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

Hi, just pointing out that some of the changes requested by emo still aren't fixed (see: pants for example)

@KingFroozy
Copy link
Contributor Author

KingFroozy commented Aug 31, 2024

Hi, just pointing out that some of the changes requested by emo still aren't fixed (see: pants for example)

They are? The shading is back to what it was, and I already pointed out that the colors are left, cause I don't want to make it inconsistent from the regular jumpsuit. If Emo suggests changing the whole CE's palette, I'll do it though. And the back vest is the same as the regular too, with which no one had any issues before (at least I didn't see any).

@UbaserB
Copy link
Member

UbaserB commented Sep 1, 2024

They are? The shading is back to what it was, and I already pointed out that the colors are left, cause I don't want to make it inconsistent from the regular jumpsuit. If Emo suggests changing the whole CE's palette, I'll do it though. And the back vest is the same as the regular too, with which no one had any issues before (at least I didn't see any).

An example would be that the pants only have two colours for shading and this is still far too simplistic

@KingFroozy
Copy link
Contributor Author

An example would be that the pants only have two colours for shading and this is still far too simplistic

Were not people complaining that a lot of colours is bad? Idk what exactly you mean, but the pants literally are the same as the common grey jumpsuit's ones, as with colours and with shading, and the only thing I did is made turtlenecks match the common.

@KingFroozy
Copy link
Contributor Author

Bits more cosmetical changes, it's barely noticeable, so I'll list them down:

  • Straps on jumpsuit on the back sprite are done the way they were in the original design of the jumpskirt.
  • Shading on upper parts of the turtlenecks are back to the original version.
  • Shading on the skirt part of the turtleneck is made consistent with regural jumpskirt.
  • Shading on skirt sprites for monkeys are redone to match the normal.

Снимок экрана 2024-09-16 141947
Снимок экрана 2024-09-16 142054
Снимок экрана 2024-09-16 142228
Снимок экрана 2024-09-16 142505

@KingFroozy
Copy link
Contributor Author

oh yeah, also, test fail is unrelated

@KingFroozy
Copy link
Contributor Author

A few basic thoughts:

I like the more saturated yellow but the contrast is quite a bit higher for things like the detailing on the turtleneck. That should be fixed.

The darker color on the pants is cool but the posted screenshots have significantly simplified and reduced shading compared to the original. The original design of the pants and all the shading is fine: the only thing i would change is the color. It also has the same high-contrast issues as the top part of the outfit, especially with the darker colors on the outlines. It's especially noticeable on the skirts (To which all of this equally applies)

I do like the new vest and i think the reduction in visual noise helps it a lot. However, not having an outline on both sides of the straps gives it some contrast issues with the yellow suit below it. I think those should be restored.

Additionally, the back of the vest has a section with very weird shading:
image

i feel like it should just continue the pattern on the rest of it and have some light shading on the lowest part.

As a final note, please submit screenshots of the art in-game. Seeing how it looks in a more natural environment is preferable to just screenshots from an editor.

Is there a chance I'm ever getting reviewed again?

Copy link
Member

@UbaserB UbaserB left a comment

Choose a reason for hiding this comment

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

One more nitpick I noticed, can you make the border/outline on the skirt a bit less dark? It's weird considering the rest of the grey doesn't have it that way and it'd look nicer if so.

@KingFroozy
Copy link
Contributor Author

One more nitpick I noticed, can you make the border/outline on the skirt a bit less dark? It's weird considering the rest of the grey doesn't have it that way and it'd look nicer if so.

done, though idk if it's even noticeable. Also, ignore the force push, it's fixed (fuck it)

@slarticodefast slarticodefast added the Undergoing Maintainer Discussion This PR is currently going through an internal discussion by the maintainer team. label Oct 3, 2024
@UbaserB
Copy link
Member

UbaserB commented Oct 4, 2024

One more nitpick I noticed, can you make the border/outline on the skirt a bit less dark? It's weird considering the rest of the grey doesn't have it that way and it'd look nicer if so.

done, though idk if it's even noticeable. Also, ignore the force push, it's fixed (fuck it)

I can't tell the difference

@KingFroozy
Copy link
Contributor Author

upd

I can't tell the difference

Now what? I'm starting to get exhausted of this tbh.

Copy link
Member

@UbaserB UbaserB left a comment

Choose a reason for hiding this comment

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

Sprite approval from me

@KingFroozy
Copy link
Contributor Author

@slarticodefast soooooo, whatcha think about finally merging it?

@slarticodefast
Copy link
Member

slarticodefast commented Oct 12, 2024

soooooo, whatcha think about finally merging it?

I just added the label because the others forgot to do that.
This needs art director approval

Edit: Oh, nevermind, Devi approved
Thank you for your contribution!

@slarticodefast slarticodefast merged commit 214a20d into space-wizards:master Oct 12, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: Sprites Should be reviewed or fixed by people who are knowledgeable with spriting or visual design. No C# For things that don't need code. Status: Needs Review This PR requires new reviews before it can be merged. Undergoing Maintainer Discussion This PR is currently going through an internal discussion by the maintainer team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants