godot icon indicating copy to clipboard operation
godot copied to clipboard

Fix load subtask not being registered leading to false progress values.

Open ajreckof opened this issue 1 year ago • 1 comments

Fixes #90076

I'm clearly not 100% sure about my solution but it works. See https://github.com/godotengine/godot/issues/90076#issuecomment-2029203547 and the previous comment for a bit more depth on why this is working

ajreckof avatar Apr 01 '24 05:04 ajreckof

Hey, I just tested this, and as far as I can tell, it solves all the issues from https://github.com/godotengine/godot/issues/90076 while maintaining the improvements from https://github.com/godotengine/godot/pull/87711 in elegant fashion.

For added context, here are a few outputs from my testing:

Case 1:

core\io\resource_loader.cpp:251 - p_path: res://Bistro_v5_2/BistroExterior.fbx
core\io\resource_loader.cpp:252 - p_original_path: 
core\io\resource_loader.cpp:253 - original_path: res://Bistro_v5_2/BistroExterior.fbx
core\io\resource_loader.cpp:254 - parent_task_path: res://scene.tscn
core\io\resource_loader.cpp:259 - INSERTED

In the regression, an empty String was being inserted in the sub_tasks HashSet. Here, original_path is set to the correct value and the sub task's progress is tracked properly.

Case 2:

core\io\resource_loader.cpp:251 - p_path: res://.godot/imported/BistroExterior.fbx-2cc48855b7169211933e362848048fa2.scn
core\io\resource_loader.cpp:252 - p_original_path: res://Bistro_v5_2/BistroExterior.fbx
core\io\resource_loader.cpp:253 - original_path: res://Bistro_v5_2/BistroExterior.fbx
core\io\resource_loader.cpp:254 - parent_task_path: res://Bistro_v5_2/BistroExterior.fbx
core\io\resource_loader.cpp:262 - NOT INSERTED

This is the scenario https://github.com/godotengine/godot/pull/87711 was meant to fix in the first place. We correctly choose p_original_path over p_path, but inserting it in sub_tasks would result in a stack overflow crash later on due to it sharing its local path with its parent task path. This new PR correctly checks for this and skips it as needed.

Case 3 would be similar to Case 2, but with original_path != parent_task_path. I can't test it now, but this is supposedly what happens in the exported build and it looks to me like that should go through with no issues as well.

Thank you for your contribution and great work!!

AlexOtsuka avatar Apr 01 '24 17:04 AlexOtsuka

Thanks!

akien-mga avatar Apr 24 '24 16:04 akien-mga