[Bugfix + Enhancement] Sword Trail fixes and enhancements#1473
[Bugfix + Enhancement] Sword Trail fixes and enhancements#1473Kenix3 merged 8 commits intoHarbourMasters:developfrom
Conversation
|
I would personally not include 3D bombs if it's not using the bomb model, I know the 3D model can't be recoloured, but in this case it's better to leave it 2D in my opinions Does this is still changing the arrow trails from the elemental arrow color? If yes, this should be an option with a checkbox, and also allow the people to choose their own arrow color trail if they want to, so they are not forced to have a trail color custom if they use custom colour for elemental arrows |
|
please split this into multiple PRs, it's a lot easier to review PRs that address one thing than a hodgepodge of fixes and enhancements. i've seen PRs like this sit unmerged for months because one of the enhancements wasn't perfect, even though the rest of the PR was great. |
|
Working on removing anything not related to trails now. |
|
Can you please rename and update the description so that it only describes what's currently in this PR? |
|
Fixed. That's my bad. I only updated the actual description and not the name of the pr itself, that's my B! |
briaguya0
left a comment
There was a problem hiding this comment.
couple things that could be cleaned up but no suggestions that would require logic changes. any comments can be addressed in a followup PR.
| } | ||
|
|
||
| //sp30 = sp30; // Optimized out but seems necessary to match stack usage | ||
| sp30 = sp30; // Optimized out but seems necessary to match stack usage |
There was a problem hiding this comment.
why was this uncommented?
There was a problem hiding this comment.
That's a good question, actually. I think it got uncommented in my mad hysteria relating to figuring out the trail code and I...guess I forgot to recomment it out.
| Color_RGB8 HammerBottomCol = CVar_GetRGB("gHammerTrailBottomCol", TrailsColorOriginal); | ||
|
|
||
| if ((CVar_GetS32("gUseTrailsCol", 0) != 0) && (this->trailType != 0)) { | ||
| switch (this->trailType) { //there HAS to be a better way to do this. |
There was a problem hiding this comment.
this would probably feel less bad if trailType was an enum, then we'd have something like
case TRAIL_TYPE_SWORD:
case TRAIL_TYPE_BOOMERANG:
etc.
There was a problem hiding this comment.
To be fair, I'd love to know how to do that. I just kinda...don't.
There was a problem hiding this comment.
good example here
probably should put that in z64effect and then instead of using u8 for trailType you could use TrailType (or whatever you end up naming the enum) as the type
| } | ||
| } | ||
|
|
||
| //dumb doo doo command to change the type of an object's blur on the fly. Link's Swords with unique trail colors. |
There was a problem hiding this comment.
this is perfectly reasonable, just adding a method because sometimes the type needs to be changed outside of z_eff_blure
There was a problem hiding this comment.
Yeah, it does with swords, otherwise changing settings mid-swing with bug out a bit. That, and if it set with other things it would just be way messier. I'm surprised I called this the dumb doo doo code instead of the massive bloat setting the proper colors for everything.
…ters#1473) * Fix Trails, add more Trail Customization * 3d Bombs; Bombchu now glow custom trail colors * 3D Seed/Nut Model, Separate Sword Slash Colors * Removed 3D Seeds/Nuts; Don't work properly * restored previous removal of sword blur code * Remove things not related to Trails * Remove fix to random color code
After Noting some issues with trails in SoH, this should take care of most bugs/inaccuracies relating to them, as well as adding & fixing customization options.
FIXES
ENHANCEMENTS
TODO