[Enhancement] MM3D style clock #713
Conversation
Just trying to get basic drawing to work before actually working on the system proper.
basic functionality now complete
removed some unused code and added some slightly better comments
formatted using clang
missing line added that caused one part of the hud to not respect the hud editor
final hours clock now lerps between yellow and red like in MM3D
delayed moving the marker backwards by one frame if time steps backwards to avoid skipping around on day transitions
There was a problem hiding this comment.
Hey! Overall, this is a really clean and isolated implementation for the clock display. In my play testing I didn't notice anything strange about the behavior. Great job!
I've left a couple suggestions/questions below, mostly just little nits and opportunities to help with code readability through comments and variable names.
|
|
||
| // This is a safety measure to delay skipping the arrow marker back a day on the frame before the | ||
| // day changes by just checking if time went backwards | ||
| if (sPreviousTimeCheck != -1 && sPreviousTimeCheck > 259200 - timeUntilCrash) { |
There was a problem hiding this comment.
Looks like you have 259200 in a few spots and I believe is the amount of seconds in 3 days. You could move this to a define for easier readability like:
#define SECONDS_IN_THREE_DAYS 3 * 24 * 60 * 60
| u16 finalTimerPos = posX - finalTimerSpacing * 4 - finalTimerSpacing / 2; | ||
| s16 i; | ||
|
|
||
| s32 percentToCrash = std::min(std::max((timeUntilCrash * 255) / 21600, 0), 255); |
There was a problem hiding this comment.
Same thing here, 21600 looks like seconds in 6 hours
SECONDS_IN_SIX_HOURS 6 * 60 * 60
| s32 finalHoursOffset = 10; | ||
| s32 finalHoursModifier = 2; | ||
| if (sFinalHoursIntro < finalHoursOffset * finalHoursModifier) { | ||
| sFinalHoursIntro++; | ||
| } |
There was a problem hiding this comment.
Seems like these values and logic are how you control the animation of the final hours sliding in. I think to help with readability you could add some comments explaining these variables and the exact intent.
| s32 currentTime = (s32)TIME_TO_MINUTES_F(gSaveContext.save.time) - 360; | ||
| if (currentTime < 0) { | ||
| currentTime += 1440; | ||
| } |
There was a problem hiding this comment.
Is this block normalizing the time to the games 6am-6am range?
Looks like the constants are minutes in 6 hours and minutes in 24 hours respectively.
But actually I don't see currenTime being used anywhere, so this code could be dropped if its not needed.
There was a problem hiding this comment.
I think these were from an earlier version of the clock and somehow managed to sneakily avoid being deleted. I should be fine to get rid of them.
| sPreviousTimeCheck = 259200 - timeUntilCrash; | ||
| } | ||
|
|
||
| s32 timeoffset = std::max(std::min(3 * 48 - (timeUntilCrash * 3 * 48) / 259200, 3 * 48), 0); |
There was a problem hiding this comment.
Looks like 48 is used in some of the position calculations and represents the width of each day section. I'd recommend using a define for this as well so that relationship is captured clearly.
| // Digital time | ||
| u16 timerSpacing = 6; | ||
|
|
||
| if (curTensHours > 0) { |
There was a problem hiding this comment.
Thoughts on always showing the leading 0 when in 24 hour time?
There was a problem hiding this comment.
Adding that because I have no strong preference and it makes sense.
| static s16 sThreeDayClockAlpha = 255; | ||
|
|
||
| static s32 sPreviousTimeCheck = -1; | ||
|
|
||
| static s32 sFinalHoursIntro = 0; |
There was a problem hiding this comment.
Either move these static vars up to the file level scope, or down into the actual hook body scope, since they are not used anywhere else.
There was a problem hiding this comment.
Chose to move these down in scope because I prefer to keep things as tightly scoped as possible where appropriate.
| OVERLAY_DISP = Gfx_DrawTexRectIA8(OVERLAY_DISP, (TexturePtr)gThreeDayClock3DSFillTex, 48, 12, | ||
| posX + 24, posY, 48, 12, 1 << 10, 1 << 10); | ||
|
|
||
| gDPSetPrimColor(OVERLAY_DISP++, 0, 0, 200, 0, 0, sThreeDayClockAlpha); |
There was a problem hiding this comment.
Move this color set down, closer to where the actual texture that uses it gets drawn.
…made 24-hour clock setting always use 4 digits. added more comments
Adds a new 3-day clock mode that mimics the style of the MM3D clock as an option in the graphics section of the enhancements menu, alongside the text-based clock. All assets are original and hand drawn.
Build Artifacts