godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.8k stars 21.13k forks source link

ResourceLoader load_threaded_get_status progress: Array = [] is always [0] #56882

Closed VP-GAMES closed 1 year ago

VP-GAMES commented 2 years ago

Bugsquad note: This issue has been confirmed several times already. No need to confirm it further.


Godot version

4.0 dev

System information

Linux Ubuntu 20.04.3 LTS

Issue description

From Godot Documentation: ● ThreadLoadStatus load_threaded_get_status(path: String, progress: Array = [])

Returns the status of a threaded loading operation started with load_threaded_request() for the resource at path. See ThreadLoadStatus for possible return values.

An array variable can optionally be passed via progress, and will return a one-element array containing the percentage of completion of the threaded loading.

But progress value is always [0]

Steps to reproduce

It is simple to reproduce, run this Project: https://github.com/VP-GAMES/Godot4InteractiveSceneChanger

Add print(_progress) in func _update_progress() [Line 53] in this script: https://github.com/VP-GAMES/Godot4InteractiveSceneChanger/blob/main/addons/interactive_scene_changer/InteractiveSceneChanger.gd

Output [0] [0] [0] [0] . . n

Minimal reproduction project

No response

nathanfranke commented 2 years ago

I can't reproduce

Edit: Hmm, it doesn't work now on my other project.

Edit 2: Okay my verdict is it is "working" but since the way progress is calculated is pretty flawed depending on the conditions it will always/most of the time be zero.

VP-GAMES commented 2 years ago

Fixed in godot 4 alpha 12 (I tested it) Output is float value from 0.0 - 1.0 [0.0] [0.1] [0.3] [0...] [0..] [0.] [1.0]

reydelatorre commented 2 years ago

this is still happening in beta2. progress[0] is always 0.

metanoia83 commented 2 years ago

Bug is still in beta 2. progress[0] output is always 0.

gelvinp commented 1 year ago

Also having this issue in beta 4.

owstetra commented 1 year ago

This bug is till in Beta 5 too

donn-xx commented 1 year ago

Can confirm, Godot 4.0 beta 6, always [0] until last moment and then it's suddenly [1]. There is no graduation between.

akien-mga commented 1 year ago

It would be useful to have a minimal reproduction project that actually exhibits the issue. It doesn't seem to be the case in the OP's project.

donn-xx commented 1 year ago

progress_bug.zip

Here's a small project. Have a look at the output. I'd expect to see 0.1, 0.2, 0.3 ... 1.0 or something like that but all one sees is 1. It may be due to the scene files it's loading being so small, but stuff them with some big gltf scene or something.

erniel commented 1 year ago

the bug is still in godot 4 beta 10

Bucham commented 1 year ago

It also happens to me in godot 4 beta 10. It goes from 0 directly to 1.

akien-mga commented 1 year ago

progress_bug.zip

Here's a small project. Have a look at the output. I'd expect to see 0.1, 0.2, 0.3 ... 1.0 or something like that but all one sees is 1. It may be due to the scene files it's loading being so small, but stuff them with some big gltf scene or something.

Thanks! I tested, adding a big .glb in scene0001.tscn to be able to have more than one load step, and I can confirm the bug. It seemingly never worked in this scenario.

https://github.com/godotengine/godot/issues/56882#issuecomment-1195423772 reported it working in 4.0 alpha 12, but I think it's a different case.

Looking at the code:

https://github.com/godotengine/godot/blob/91713ced81792b10fdc9367b7f355738e5d52777/core/io/resource_loader.cpp#L389-L409

For a resource with dependencies, the progress value might be valid and report basically the dependency count. But in the else case, load_task.progress seems to always be 1.0. And indeed I don't see any code setting progress to a value different from 1.0.

reduz commented 1 year ago

Same bug? #65380

akien-mga commented 1 year ago

Same bug? #65380

Indeed. @gotnospirit has a proposed solution there, would be good to have a PR unless you think it's not the right approach. https://github.com/godotengine/godot/issues/65380#issuecomment-1242892124

Edit: Did some testing, #65380 is not the exact same problem as the MRPs in this thread, though it's all related. In #65380, multiple ext_resources are being loaded and they don't report the progress. So @gotnospirit's proposal to add the missing assignment to progress there seems to solve #65380 indeed.

But it doesn't solve this MRP: https://github.com/godotengine/godot/issues/56882#issuecomment-1377063435 Which is also loading an ext_resource, but a single, heavy one. It only reports 0 when not loaded and 1 when loaded, nothing in between, which seems to be what some are asking for here (for resources which take a long time to load, not due to dependencies but just due to being heavy).

I tested again @VP-GAMES's MRP in https://github.com/godotengine/godot/issues/56882#issue-1106345653, and that one seems to work somewhat OK as of 4.0 beta 10 (reports 0.25 and 0.5 progress values while loading). And here @gotnospirit's proposed fix for #65380 actually makes things worth (reports 0.83333 and 0.583333 as progress values, not sure what it maps to).

reduz commented 1 year ago

Besides fixing #65380, I am not sure this is a bug and I don't think its possible to get reliable progress in all types of scenes. Let me explain how the threaded loading algorithm works so you understand why:

Scenes contain two types of resources:

To get the perfect load %, you would need to amount to all the tree of external resources until the very end. The problem is that, this is not know from the beginning because threaded loading has a limit on how many resources are being loaded at the same time (otherwise loading could spawn 698 threads and kill the loading performance).

This means that, at the beginning of the load, you only know the resource being loaded has N direct external resources, but not how many subresources they have respectively. So, if you are loading a resource that is a single scene that has a subscene in another file, you don't know all the resources the subscene has, so your loading progress will be slower.

The current algorithm should work going from 0 to 1 reliably because it splits every scene progress in

But it will not go at constant speed, so it may appear it does not work.

The "correct" way to solve this would be, at export time, check the dependency chart of every resource and save it in some metafile, so you can tell in advance how many "steps" are needed to load the resource. This is a lot of work and beyond 4.0 expected feature set. It will probably get added at some point in the future.

Zireael07 commented 1 year ago

The current algorithm should work going from 0 to 1 reliably because it splits every scene progress in 0-50% - All external resources 50-100% - All internal resources

Clearly, from this issue, it doesn't work like that. It's stuck at 0 and then suddenly jumps to 100.

I do not care that the rate is not constant. I don't care if the algorithm is not perfect, that the jumps are uneven, but I need the loading bar to make sense to my players and not look like the game's stuck immediately.

donn-xx commented 1 year ago

Perhaps some kind of 'fake' progress that goes log slow until the recursion starts popping back up to the top level? I wouldn't mind an overall progress (at the top level) and then a different behaviour while Godot is plumbing the unknown depths of ones scene trees.

realkotob commented 1 year ago

@reduz Your suggested solution is perfect but it might be over engineered in this case, as @Zireael07 pointed out we do not need a perfect percentage.

Just providing a "loaded resources" number and a "total loaded + pending" number (or a percentage based on their ratio) would be completely adequate for showing to players in a loading screen. This number will vary since the pending files will be changing, but that's ok.

In many games and applications we are shown "Loading 2/10 files" which then changes to "Loading 42/389" etc. In unity this is shown when building an executable, the number of files is variable and also you usually only see 2 number changes of this before it jumps to the next resource category.

image

So many games do not bother to show accurate percentages, like in The Sims where they just show nonsensical text. Sure there is a progress bar, but in some Sims games it jumps around and that does not matter. What matters is being able to show progress. ANY progress.

Reporting the total number of loaded files, even without giving an estimate of the remaining files, lets developers set their own thresholds for when to show a different non-sensical text to the users.

image

I believe the old Windows file copy/download dialog is also a good notorious example, it was jumping around all the time and it still made sense on the whole.

This is an image from Windows XP, but even on Windows 11 now the file counter is not stable, although more stable because it enumerates them before it starts.

Again, this is not perfect and people might want an accurate progress percentage so they can use it for analytics reporting, preparing cutscenes or gameplay, but that is not the most common usecase. The most common use case is just showing some form of progress for the user. Currently there is 0 of that.

image

I have worked on a lot of loading screens in my short career, the main concern was always making the user feel the game is not stuck. There was almost never any other usecase.

In the case of godot, the best I could do for a mobile game is to measure how much time it takes to load on my phone, then use that time as a benchmark and use a timer to update at intervals along this duration. And if it reaches the expected time and it's not finished loading it just stays stuck at 90%.

I have also worked on Facebook and Viber games, and by the way this is also how they do it. (Just let it get stuck at the last part).

This is why I say that even reporting the current total number of loaded assets is enough, because that lets us do this same workaround but make it actually accurate, so that 100 resources shows 12%, 220 files shows 30%, etc.

FB Instant Games: image

TokisanGames commented 1 year ago

Besides fixing #65380, I am not sure this is a bug and I don't think its possible to get reliable progress in all types of scenes.

The bottom line is we need to give users some indication that the game has not crashed and is still loading. That's the whole point. A crashed game results in a refund, a bad review, and additional lost sales.

In Godot 3, the ResourceInteractiveLoader identified some 130 loading stages in my game and was able to give a finely detailed percentage. Now in Godot 4 it goes from 0 to 1. That is a major downgrade and is far too coarse. Continuous progress with 100% accuracy is unnecessary, and wasn't provided in 3.

The current algorithm should work going from 0 to 1 reliably because it splits every scene progress in 0-50% - All external resources 50-100% - All internal resources

There's two steps right there. You could get a list of all external sources and files that contain the internal resources and take a percentage of that. All the scene files I've seen have a list of resources right at the top. That could be read first. Or you can also get all of the file sizes of the files and the resources in the PCK and calculate based upon bytes loaded off the disk.

Even 25% chunks would be helpful. But surely there is a more reasonable solution. What did GD3 RIL do? Why can't you do that again?

realkotob commented 1 year ago

What did GD3 RIL do? Why can't you do that again?

@TokisanGames Godot 3 did not have threaded resource loading iirc, which is the differentiator here that adds complexity.

TokisanGames commented 1 year ago

Godot 3 did not have threaded resource loading AFAIK, which is the differentiator here that adds complexity.

ResourceInteractiveLoader was the progress providing loader in GD3. It was removed in GD4, and now the only class available with an interactive loading poller is the threaded function of ResourceLoader. However it doesn't actually provide what the documentation says it provides.

It doesn't matter to me, and probably most other gamedevs, whether the loader is technically threaded or a single threaded process. The only thing that matters is an engine provided mechanism for providing progress to the end user. This ticket and the several other duplicate issues are not about threads, but about progress indication. The loader defaults to a single thread anyway. Multithreaded is broken #62159 #66023.

Right now on my game in beta12, my title screen takes 13-17 seconds to load. I'm using single threaded mode and I get 0 for most of that time, then the last two frames are .75 and 1. So basically no progress.

In multithreaded mode my game may or may not load. I just tried three times. 2 failures stuck in a loop, one partial load. The game ran but half the assets were missing. In all three instances, progress started and ended at .874625. Never 0 or 1. So no progress and no load.

eh-jogos commented 1 year ago

Still happening on 4.0rc3 image

The current algorithm should work going from 0 to 1 reliably because it splits every scene progress in

* 0-50% - All external resources

* 50-100% - All internal resources

The algoritm @reduz described in a comment above does not seem to be working or I should have seen at least 0.5 somewhere. What I'm loading is a huge tscn with lot's of external resources, so at least in the 0-50% range I should be getting something, right?

I agree with the other comments, even if it was just counting the list of external resources in the tscn/tres and not going any deeper than that it would be a lot better. I always had the impression the number of "stages" in Godot's 3.x ResourceInteractiveLoader was related to the list of external resources as they were usually close when loading scenes with it.

I also agree with the other comments this is a big "break" in relation to 3.x, is there any chance that there will be any improvements in this before 4.0 stable? Or is this going to be in a later release in favor of a more complex fix?

Even if it only shows progress for .tscn and .tres files I think it's already an improvement.

Edit: BTW this is far from an MRP but the prints come from a project that is open sourced here https://github.com/quiver-dev/template-beat-em-up, just clone it and run the game and watch the prints.

The main menu loads the first stage in the background but if the player starts the game before it is finished loading, it shows a loading bar between transitions, which currently is always on 0 and suddenly fills out, but the prints are easier to track for this case.

maximkulkin commented 1 year ago

Ok people. I've spent some time to debug this and I can confirm there is a problem. First I thought it was an issue with the memory synchronization between threads (because progress on each thread it's just pointer to a float). I did an experiment on replacing all progress pointers from pointer to a float to pointer to std::atomic. That did not help. Though I think it will backstab you if you try to load resources using subthreads.

Then I did some more tracing and found out that the progress for loading external resources is just not reported: Hello? Where is the progress report? It does report progress for internal resources, though "resources_total" value (obtained from "load_steps" field).

After fixing this (in both Text and Binary ResourceLoader) it starts showing progress, but there's another issue. Documentation for ResourceLoader::load_threaded_get_status() states:

An array variable can optionally be passed via [param progress], and will return a one-element array containing the percentage of completion of the threaded loading.

Notice "containing the percentage of completion". But if you look at it's implementation it will be obvious that it returns value in range from 0 to 1. Not an end of the world, but some consistency would be nice.

And lastly, it's all good and dandy, unless you want to use subthreads. This part never worked for me. If my scene loads fine in single threaded mode, it produces errors if I do subthreads. And the reason for that is that both Text and Binary resource loaders in case of subthreads fire a request to load an external subresource, but they never wait for them to finish loading. And when those loaders proceed with initializing internal subresources or main resource, in a lot of cases it ends up having external resources not loaded yet and thus complains about missing resources. There's just no waiting for them. Not sure if it was ever tested. And I believe once it will work, then it will start having those thread memory synchronization/stale memory issues.

Zireael07 commented 1 year ago

Notice "containing the percentage of completion". But if you look at it's implementation it will be obvious that it returns value in range from 0 to 1. Not an end of the world, but some consistency would be nice.

In programming, 0-1 indeed often is used to represent percentage. This should be clarified in the docs I think, it's not an implementation issue as such IMHO.

maximkulkin commented 1 year ago

@RandomShaper I checked your PR and it's a great improvement. I especially like moving logic on spawning new load threads from particular resource loaders into central ResourceLoader. However, that PR won't fix the external resources counting problem, which seem to be the case here. I was holding my PR to figure out subtask loading, but since you've already took care of it, I will post my PR.

Miguelito223 commented 1 year ago

is happent the same to me

matthew1006 commented 9 months ago

This bug is still present in 4.2.1 web exports.

extends Node

@export_file("*.tscn") var scenePath
@onready var progressBar := $%ProgressBar as ProgressBar

func _ready():
    ResourceLoader.load_threaded_request(scenePath, "PackedScene")

func _process(_delta):
    var progress = []
    var status = ResourceLoader.load_threaded_get_status(scenePath, progress)
    print(progress)
    if status == ResourceLoader.THREAD_LOAD_LOADED: # Finished loading.
        get_tree().change_scene_to_packed(ResourceLoader.load_threaded_get(scenePath))

    elif status == ResourceLoader.THREAD_LOAD_IN_PROGRESS:
        progressBar.set_value(progress[0])

image