Add manual time sync option #25
Conversation
|
It's strange that this opportunity didn't exist before. |
You can already do that. The aim here is for setting static time via the API while timed updates are disabled. |
|
I appreciate the effort to keep up with main, but it's unnecessary, and the process is incorrect. You should only rebase. Never merge or cherry pick commits into your branch. I can automatically rebase on merging the PR, so there's no need unless there's a major change or I ask you too. As it is, I'd like you to do an interactive rebase, drop the last commit and force push your branch into the PR. As for the code, I'm working on Terrain3D collision and will review this later. I think there might be a better approach, but I need to look at it and will let you know so you can adjust it. |
|
I'll rebase this soon, thank you! |
There was a problem hiding this comment.
Thanks for working on this. I would like some changes to the design of this implementation.
Some important background:
TODv1 was designed for updating in _process. I changed it to update based on a timer of update_interval. There is some legacy code for the former that doesn't make sense for the latter and should be removed.
Delta originally came from _process, but now I manually calculate when I wrote _on_timeout(). So a key question is what is delta used for?
It looks like delta does 2 things:
- It progresses time forward. If a delta of 1000ms occurs, then time moves forward 1s.
- A manual timer is tracked for updating celestial positions on a slower schedule than every frame like the rest of the system. This manual timer is now redundant and can be removed.
| @@ -103,6 +103,7 @@ func set_show_physical_sky(value: bool) -> void: | |||
| @export_range(0.0, 24.0) var current_time: float = 8.0 : set = set_current_time | |||
| @export_range(-1440,1440,1) var minutes_per_day: float = 15.0 : set = set_minutes_per_day | |||
| @export_range(0.016, 10.0) var update_interval: float = 0.1 : set = set_update_interval | |||
| @export var manual_time_management: bool = false : set = set_manual_time_management; | |||
There was a problem hiding this comment.
update_in_game and update_in_editor and both disabled (manual) already define three states. This variable is redundant with both disabled. So rather than tracking it here and the support functions, all we really need is an update() function that can be called at any time, even when both update settings are enabled, to refresh the system without adjusting the time.
| __time_process(0) | ||
| __repeat_full_cycle() | ||
| __check_cycle() | ||
| __set_celestial_coords() |
There was a problem hiding this comment.
This is a good start on the update function, but it also is redundant with _on_timeout(), so we should combine them. Instead, lets make these changes:
_on_timeout()is stripped down to only calculate delta and_last_update, then callsupdate(delta). This means delta will only be calculated from calls to _on_timeout(). Any additional calls to update() won't change the delta calculation from the timer.- A new
update(delta: float = 0.0)function has the rest of the _on_timeout() function. - The whole system_sync section should be skipped if
is_approx_zero(delta) __time_process()can be renamed to__progress_time()as that's what it does.- There's a problem if system sync is enabled, which calls
__get_date_time_os(). That will call set_day, set_month, set_year which is going to fire off day/month/year signals, even if they haven't changed. Those functions should have a check if changed before emitting. - All of the
__celestials_update_timerstuff can be removed. Just call__set_celestial_coords()every time update() runs. emit(time_changed)is currently inset_total_hours. I think it should be moved toupdate(), andset_total_hourscallsupdate(). Though time isn't always technically changing, it is an adequate generic "update" signal that is safe to call every micro second.
Now the timer can fire off and update as normal to progress time, and we can do manual zero-delta updates anytime without changing the time.
| @@ -130,28 +138,28 @@ var year: int = 2025: set = set_year | |||
| func set_total_hours(value: float) -> void: | |||
| total_hours = value | |||
| emit_signal("time_changed", value) | |||
| if Engine.is_editor_hint(): | |||
| if is_editor_hint_or_manual(): | |||
| __set_celestial_coords() | |||
There was a problem hiding this comment.
Here reverting is not right. There's no need for the editor hint at all. This should call update(), and the signal be moved to that function.
func set_total_hours(value: float) -> void:
total_hours = value
update()
| func set_manual_time_management(value:bool) -> void: | ||
| manual_time_management = value | ||
| if tod: | ||
| tod._manual_time_management = value |
|
|
||
| func manual_time_update() -> void: | ||
| if tod: | ||
| tod.manual_time_update() |
There was a problem hiding this comment.
Change both to just update()
|
|
||
| func is_editor_hint_or_manual() -> bool: | ||
| return Engine.is_editor_hint() or _manual_time_management |
|
|
||
| @export var update_in_game: bool = true : | ||
| set(value): | ||
| update_in_game = value | ||
| if not Engine.is_editor_hint(): | ||
| if not is_editor_hint_or_manual(): |
There was a problem hiding this comment.
Revert. Editor hint is appropriate.
| @@ -28,7 +31,7 @@ var _last_update: int = 0 | |||
| @export var update_in_editor: bool = true : | |||
| set(value): | |||
| update_in_editor = value | |||
| if Engine.is_editor_hint(): | |||
| if is_editor_hint_or_manual(): | |||
There was a problem hiding this comment.
Revert. Editor hint is appropriate.
| if (Engine.is_editor_hint() and update_in_editor) or \ | ||
| (not Engine.is_editor_hint() and update_in_game): | ||
| if (is_editor_hint_or_manual() and update_in_editor) or \ | ||
| (not is_editor_hint_or_manual() and update_in_game): |
There was a problem hiding this comment.
Revert. Editor hint is appropriate.
| __set_celestial_coords() | ||
|
|
||
|
|
||
| func set_day(value: int) -> void: | ||
| day = value | ||
| emit_signal("day_changed", value) | ||
| if Engine.is_editor_hint(): | ||
| if is_editor_hint_or_manual(): | ||
| __set_celestial_coords() |
There was a problem hiding this comment.
Change to:
func set_day(value: int) -> void:
if day != value:
day = value
emit_signal("day_changed", value)
update()
Change all the rest of the set_* functions you changed below. The check if changed is only needed if emitting signals. They can all always update.
Adds flag and method for manual time sync.
To use, you disable "Enable game time"
and enable "Manual Time Management"
and then call
manual_time_update()after programmatically setting the time in game.In my game, the time only changes when the characters move in the overworld for instance.