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

Cosmetics Editor entry for Ivan the Fairy #3718

Merged

Conversation

skyyoshi86
Copy link
Contributor

@skyyoshi86 skyyoshi86 commented Dec 19, 2023

image

Added Cosmetics Editor entry to change the color of Ivan the Fairy

Build Artifacts

Copy link
Contributor

@jbodner09 jbodner09 left a comment

Choose a reason for hiding this comment

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

The changes to the CMakeSettings and MODDING.md seem unrelated to Ivan and shouldn't be included in this PR.

Reverted a correction to MODDING.md (case-sensitive broken link?) as this is not a change related to this PR
Deleted CMakeSettings.json as this is not a change related to this PR.
@skyyoshi86
Copy link
Contributor Author

The changes to the CMakeSettings and MODDING.md seem unrelated to Ivan and shouldn't be included in this PR.

TY for Review! I'm learning. CMakeSettings.json file was generated/changed automatically, and now I don't know if I need to delete it. I now deleted it. Was it correct to delete it?

@jbodner09
Copy link
Contributor

Yeah, it was most likely auto-generated by your IDE, but shouldn't have been. If other people run into this, it would be easy enough to add it to the git ignore list, but at least on my machine, the file doesn't exist, so I'm curious what your setup is.

@skyyoshi86
Copy link
Contributor Author

Yeah, it was most likely auto-generated by your IDE, but shouldn't have been. If other people run into this, it would be easy enough to add it to the git ignore list, but at least on my machine, the file doesn't exist, so I'm curious what your setup is.

TY! I'm on Windows 10 using Visual Studio 2022. As I kept getting an "could not clone vcpkg" when trying to compile, I was told to use "& 'C:\Program Files\CMake\bin\cmake' -S . -B "build/x64" -G "Visual Studio 17 2022" -T v142 -A x64 -DVCPKG_ROOT="build/x64/vcpkg" -DBUILD_REMOTE_CONTROL=1" command when compiling. I was told that "It's trying to update the vcpkg that came installed with Visual Studio, rather than making a separate vcpkg clone just in the project."

@skyyoshi86
Copy link
Contributor Author

Am I missing a step for this PR?

```
Copy link
Contributor

Choose a reason for hiding this comment

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

anyone know what this change is? it doesn't appear to be newline/not

Copy link
Contributor

Choose a reason for hiding this comment

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

Well it is, but github apparently displays it differently for markdown files than it does for code files. I checked out this branch and opened the file up in a hex editor just to confirm:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Since the other markdown files end in newlines, it should be fine to leave it.

Comment on lines 865 to 868
if (CVarGetInteger("gCosmetics.Ivan_IdlePrimary.Changed", 0)) {
Color_RGB8 ivanColor1 = CVarGetColor24("gCosmetics.Ivan_IdlePrimary.Value", (Color_RGB8){ 255, 255, 255 });
gDPSetPrimColor(dListHead++, 0, 0x01, (u8)ivanColor1.r, (u8)ivanColor1.g, (u8)ivanColor1.b,
(u8)(this->innerColor.a * alphaScale));
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the body and particle colors are the same (both default green or the cosmetic color), instead of doing this lookup twice in each draw func, you can instead do it once in the EnPartner_Update func, and have it set this->innerColor and this->outerColor to the right color, then the two draw funcs can be reverted to use those values again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the body and particle colors are the same (both default green or the cosmetic color), instead of doing this lookup twice in each draw func, you can instead do it once in the EnPartner_Update func, and have it set this->innerColor and this->outerColor to the right color, then the two draw funcs can be reverted to use those values again.

image

TY for your help and feedback :D

  1. How can I get the colors from Cosmetics Editor with a function similar to CVarGetColor24() that returns a Color_RGBAf value instead of returning a Color_RGB8 value?

-or-

  1. Should I instead modify the declarations of this->innerColor and this->outerColor to be of type Color_RGB8 so I can assign the Cosmetic Editor colors via the CVarGetColor24() function?

EDIT:

  1. Should I keep using my newly created variables ivanColor1 and ivanColor2 of type Color_RGB8 as an alternative to assign the colors if they have been changed in the Cosmetics Editor?

Thank you in advance :)

Copy link
Contributor

Choose a reason for hiding this comment

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

You can assign the individual color properties one by one, which will handle the casting from int to float.

Color_RGB8 idlePrim = CVarGetColor24(...);
this->innerColor.r = idlePrim.r;
this->innerColor.g = idlePrim.g;
...

Or if you are daring, you can change the Color_RGBAf in the EnPartner struct to be Color_RGBA8. Either way is fine.

Please review carefully. I made these changes blindly as I figure out why I can't compile it in my machine anymore. Sorry!
Copy link
Contributor

@Archez Archez left a comment

Choose a reason for hiding this comment

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

Awesome, looks good now! 🙂

Co-authored-by: briaguya <70942617+briaguya-ai@users.noreply.github.com>
Co-authored-by: briaguya <70942617+briaguya-ai@users.noreply.github.com>
garrettjoecox and others added 2 commits February 1, 2024 20:07
Co-authored-by: briaguya <70942617+briaguya-ai@users.noreply.github.com>
Co-authored-by: briaguya <70942617+briaguya-ai@users.noreply.github.com>
@garrettjoecox garrettjoecox merged commit 351a511 into HarbourMasters:develop Feb 2, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants