-
-
Notifications
You must be signed in to change notification settings - Fork 49
Description
First off, thanks for doing this! Using this has fixed the flakiness I was experiencing with the script not always being ready.
- When reading the docs for
progress()it says:
returns a number between 0 and 100.0 to represent the current script pipeline progress,
But that seems odd to me. A value of 100.0 means "100 percent loaded" but that is a UI concern. If anything, I feel this should return progress as a float between 0.0 and 1.0.
-
num_loaded_scripts()should arguably be namedloaded_script_count()andnum_processing_scripts()->processing_script_count(). See my argument/rationale from an issue in Bevy: Addnum_entities()to World bevyengine/bevy#19780 (comment). It's not an objective choice, but take a look anyway. -
processing_batch_completed()should arguably be namedis_processing_batch_completed(). You can follow breadcrumbs here RenameTimer::pausedtoTimer::is_pausedandTimer::finishedtoTimer::is_finishedbevyengine/bevy#19110 but tl;dr; is that most Rust functions seem to use this format. -
Thinking about
progress()and my comments in 1., I wonder if you should actually just remove it entirely. Users can already get all the data by callingnum_loaded_scripts()andnum_processing_scripts()and I personally wouldn't ever need to know the percent progress: just whether or not it's done. That is, it really feels more like a UI concern. Up to you though! -
Regarding the use of the term
batchinprocessing_batch_completed(), what is the purpose? As a user, I don't feel like I really know anything about batches here so it was confusing to me looking at this method name and wondering if it's what I want. If it was justprocessing_completed()(read:is_processing_completed()), would that lose any information?