makspll / makspll/bevy_mod_scripting
ScriptPipelineState progress quality of life changes
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 649
- Forks
- 59
- PR merge metrics
- No merged PRs in 30d
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: https://github.com/bevyengine/bevy/pull/19780#issuecomment-2994554024. 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 https://github.com/bevyengine/bevy/issues/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?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the ScriptPipelineState API and its documented progress(), num_loaded_scripts(), num_processing_scripts(), and processing_batch_completed() methods. Review the linked Bevy discussions before deciding the progress semantics and naming. Done means the API direction is agreed and the affected documentation and callers are updated consistently.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design, game-dev
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100