Day/night clock: the sun, sky and light follow the hour (ENG-F82) #130
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "khyretos/claude/day-night-2"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Requested by Kees · project thread
Rebased onto main after #127 (Lua pathfinding), which touches the same ScriptModule lines. All 1316 tests pass locally.
What (ENG-F82: day/night cycle)
kke::DayNight(kke/DayNight.h), pure maths: the sun rises in the east atsunrise, crosses the south atnoonElevationand sets in the west atsunset. For any hour it gives a gradient sky (zenith, horizon, sun colour and glow), the key light's direction, colour and strength, and the exposure, keyed on the sun's elevation from noon through golden hour, sunset and dusk to night. By night the key light becomes a pale blue moon, opposite the sun and at least 25° up.advance(dt)moves the clock and returns the hours, dawns and dusks it passed, in order and across midnight.apply()lights aLighting, and changes the sky only when asked.SkyResolverprefilter reflections and ambient SH again (plus a device wait for the upload), soScriptClockmoves the sun every frame but refreshes the sky at most every 0.5 s, and only when a colour changed by more than 3%.clock.*(kke::ScriptClock):start{hour, dayLength, noon, sunrise, sunset},set(hour),pause,resume,stop,hour,time→ "07:30",isDay,running,sun→ direction, elevation,dayLength. HooksDawn,DuskandHour(e.hour). Choosing a mood stops the clock, as does unloading the script that started it.Tests
test_day_night.cpp(5 tests): sunrise is east at 0°, noon is south atnoonElevation, sunset is west, hours wrap, and a short low winter day works. Light: a white sun and blue sky at noon, a warm weaker sun at golden hour, blue moonlight with more exposure at night, and no jump in key strength minute by minute over 24 h.advanceorder around dawn, dusk and midnight, with a huge step capped at one day.applyturns a mood's fill off and leaves the sky alone unless asked.skyChangedsays no for a midday minute and yes for an hour at dusk. The Lua API end to end: hooks fire in order, plus pause, set, time text, sun direction, release on unload, and NaN refused.kke_tests: 1316 passed after the rebase, no warnings;mkdocs build --strictclean; the CI include and dependency checks pass.🤖 Generated with Claude Code
https://claude.ai/code/session_019rkS6wq7fV9fWdiLwxyyc7
Title
Day/night clock: the sun, sky and light follow the hour (ENG-F82)
User description
Requested by Kees · project thread
Rebased onto main after #127 (Lua pathfinding), which touches the same ScriptModule lines. All 1316 tests pass locally.
What (ENG-F82: day/night cycle)
kke::DayNight(kke/DayNight.h), pure maths: the sun rises in the east atsunrise, crosses the south atnoonElevationand sets in the west atsunset. For any hour it gives a gradient sky (zenith, horizon, sun colour and glow), the key light's direction, colour and strength, and the exposure, keyed on the sun's elevation from noon through golden hour, sunset and dusk to night. By night the key light becomes a pale blue moon, opposite the sun and at least 25° up.advance(dt)moves the clock and returns the hours, dawns and dusks it passed, in order and across midnight.apply()lights aLighting, and changes the sky only when asked.SkyResolverprefilter reflections and ambient SH again (plus a device wait for the upload), soScriptClockmoves the sun every frame but refreshes the sky at most every 0.5 s, and only when a colour changed by more than 3%.clock.*(kke::ScriptClock):start{hour, dayLength, noon, sunrise, sunset},set(hour),pause,resume,stop,hour,time→ "07:30",isDay,running,sun→ direction, elevation,dayLength. HooksDawn,DuskandHour(e.hour). Choosing a mood stops the clock, as does unloading the script that started it.Tests
test_day_night.cpp(5 tests): sunrise is east at 0°, noon is south atnoonElevation, sunset is west, hours wrap, and a short low winter day works. Light: a white sun and blue sky at noon, a warm weaker sun at golden hour, blue moonlight with more exposure at night, and no jump in key strength minute by minute over 24 h.advanceorder around dawn, dusk and midnight, with a huge step capped at one day.applyturns a mood's fill off and leaves the sky alone unless asked.skyChangedsays no for a midday minute and yes for an hour at dusk. The Lua API end to end: hooks fire in order, plus pause, set, time text, sun direction, release on unload, and NaN refused.kke_tests: 1316 passed after the rebase, no warnings;mkdocs build --strictclean; the CI include and dependency checks pass.🤖 Generated with Claude Code
https://claude.ai/code/session_019rkS6wq7fV9fWdiLwxyyc7
PR Type
Enhancement
Description
Adds day/night cycle with sun, sky and light following the hour
Lua clock.* API controls time, triggers Dawn/Dusk/Hour hooks
Sky refreshes only when visibly changed to save CPU
Diagram Walkthrough
File Walkthrough
6 files
Implements day/night cycle math for sun, sky and lightLua clock API with hooks and sky throttlingRegisters ScriptClock with Lua VM and update loopDefines DayNight class with lookAt and advanceDefines ScriptClock wrapper for Lua integrationDeclares ScriptClock pointer in ScriptModule1 files
Tests sun path, lighting transitions and Lua hooks3 files
Updates ENG-F82 status to completed clock featuresDocuments time of day usage and Lua examplesAdds clock module to Lua API reference table2 files
Adds DayNight and ScriptClock source filesIncludes test_day_night.cpp in test executablePR Reviewer Guide 🔍
(Review updated until commit
6723e07f1e)📋 Focus areas: 🔴 3 open · ✅ 2 handled. Every push re-checks them; a handled area moves to Resolved findings at the bottom.
Possible Bug in Advance Logic
In the
advancefunction, when calculating events that occurred during a time step, the code checks ifm.at > from && m.at <= to. However,fromis the current hour (e.g., 5.5) andtois the new hour (e.g., 6.5). If an event happens exactly at the start time (which shouldn't happen due to floating point precision but theoretically), it would be missed. More critically, the logic usesstd::floor(wrap24(m.at) + 1e-4f)to get the hour for Hour events. This epsilon addition can cause issues near midnight boundaries or whenm.atis very close to an integer hour from below, potentially rounding up incorrectly. For example, ifm.atis 6.0000001, adding 1e-4f makes it 6.0001, floor gives 6. But ifm.atis meant to represent the exact moment of crossing, the epsilon might push it into the next hour bucket incorrectly in edge cases. The conditionm.at > fromshould probably bem.at >= fromto handle the case where the clock starts exactly on an hour boundary and advances.Missing Error Handling for Lua Hooks
In the
updatefunction, when calling hooks likevm.callHookWith, there is no check to see if the hook actually exists or if the call succeeded. If a script registers a hook with a name that doesn't match "Dawn", "Dusk", or "Hour", or if the hook function itself throws an error, the engine might crash or behave unpredictably. The code assumes the hook will always be found and executed successfully. It would be safer to wrap these calls in error handling or check if the hook exists before attempting to call it.Unused Member Variable
The member variable
m_hairRootsis declared in the header file but its usage is not fully clear from the diff. While it is used inCharacterCreatorModule.cppto store the number of roots requested by a hairstyle, the initialization and update logic for this variable is only visible in the cpp file. If this variable is intended to be exposed or used elsewhere, it should be documented. If it's purely internal, consider making it private or removing it if it's not necessary.✅ Resolved findings
engine/src/ScriptClock.cpp:50-55
Potential Resource Leak
In
ScriptClock::release, the function setsm_active = falseandm_clock.running = falseonly ifm_owner == source. If a script unloads but does not match the owner (which shouldn't happen in normal flow but could occur ifm_owneris stale or logic diverges), the clock remains active and continues to update lighting every frame. More critically, ifm_appbecomes null while the clock is active (e.g., application shutdown sequence), subsequent calls toupdatewill dereference a null pointer inlight()at line 17 (if (!m_app || !m_active) return;). While there is a check for!m_app, them_lastSkyand other member variables are not cleared, potentially leading to undefined behavior if the clock is reused or if the application state is inconsistent. It is recommended to explicitly clearm_lastSkyand reset other state inreleaseto ensure a clean shutdown.engine/src/DayNight.cpp:119
Possible Bug in Advance Logic
In the
advancefunction, when calculating events that occur betweenfromandto, the code usesstd::floor(wrap24(m.at) + 1e-4f)to determine the hour. The epsilon1e-4fis added before flooring. This can cause incorrect hour values when the wrapped time is extremely close to an integer from below (e.g., 6.00009). For instance, ifm.atwraps to 6.00009, adding 1e-4f makes it 6.00019, which floors to 6. However, if the logic intended was to capture the exact integer hour reached, this epsilon might shift the boundary slightly or behave unexpectedly near midnight transitions where floating point precision is critical. A more robust approach would be to use a small epsilon for comparison rather than shifting the value before flooring, or to ensure the wrapping logic aligns perfectly with integer boundaries without arbitrary offsets.PR Code Suggestions ✨
No code suggestions found for the PR.
🧹 Lint report · ❌ 38 problems in 6 checks · 🧹 3 formatting checks to tidy
Nothing here blocks the merge (report only).
MARKDOWN · markdownlint: first lines of the output
CPP · cppcheck: first lines of the output
YAML · yamllint: first lines of the output
BASH · shellcheck: first lines of the output
DOCKERFILE · hadolint: first lines of the output
REPOSITORY · trivy: first lines of the output
🧹 Formatting only (layout, no behaviour; the nightly lint-fix job applies these as a PR): YAML · prettier (57 files checked), BASH · shfmt (11 files checked), JSON · prettier (147 files checked)
✅ Clean: actionlint, git_diff, gitleaks, grype, jsonlint, secretlint, stylelint, trufflehog, xmllint
MegaLinter on soucouyant · full logs in the Actions run · comment
/improvefor AI fix suggestions