Loading a zip resource pack inside a resource pack fails with "ERROR: Condition "!zfile" is true. Returned: false" after v3.6.beta4

[Kersoph]

Tested versions

4.x

• Does not work in 4.5 dev 4 (bug squad edit)

3.x

• Works in v3.5.3.stable.official.6c814135b
• Works in v3.6.beta1.official.632a544c6
• Works in v3.6.beta2.official.68c507f59
• Works in v3.6.beta3.official.21ab700f2
• Works in v3.6.beta4.official.93ac21cf8
• Does not work in v3.6.beta5.official.2a347ab86
• Does not work in v3.6.stable.official.de2f0f147

System information

Windows 10

Issue description

In Godot 3.6 Beta 4 and earlier, it was possible to load a resource pack (e.g. with ProjectSettings.load_resource_pack("...")) that can contain additional resource packs to load. This is used for modding or project organisation (where one resource pack is a complete mod/dlc that can contain additional resources to load at a specific location or with a specific priority / order / logic).

While it is certainly possible to develop a modding / organisation strategy without packs in packs, it breaks all mods and projects that use such a system.

Steps to reproduce

1. Open the MRP.
2. Run the main scene.
3. The window shows an OK image if everything could be loaded. Nothing if there was a problem (and error messages in the console)

Create your own:
A. zip a resource/folder (e.g. a .png)
B. zip the created zip (with other files you need)
C. Load the resources with ProjectSettings.load_resource_pack(). This loads all resources from step B into the project.
D. Try load the content of the internal zip from step A with ProjectSettings.load_resource_pack(). This fails in newer versions.
E. (The .png can not be loaded if step D failed)

Error message at step D:

Godot Engine v3.6.stable.official.de2f0f147 - https://godotengine.org
OpenGL ES 3.0 Renderer: NVIDIA GeForce
Async. shader compilation: OFF

ERROR: Condition "!zfile" is true. Returned: false
  at: try_open_pack (core/io/file_access_zip.cpp:171) - Condition "!zfile" is true. Returned: false

Minimal reproduction project (MRP)

LoadRegression.zip

I've added variables for almost each step so you can set breakpoints and directly inspect step by step what happens.
The last line shows that the .zip from step A can be loaded directly without problems.

[Kersoph]

At first sight in the diff beta4 - beta5, I only see a small change around file_access_zip.cpp
But would be weird if this is the issue.

[precup]

Bisected to the cherry pick that originates from #91210, cc @Faless for context

[lawnjelly]

If I change the get_position() code to this, fascinatingly the not equal condition hits, with pos as 4, and pos2 as 294. Not sure why yet.

uint64_t FileAccessZip::get_position() const {
	ERR_FAIL_COND_V(!zfile, 0);
	ZPOS64_T pos = unztell64(zfile);
	off_t pos2 = unztell(zfile);
	
	if (pos != pos2)
		return pos;
	
	return pos;
}

[lawnjelly]

• I wonder whether the problem is that the zip file has been created in a 32 bit format, but cannot be read with 64 bit format?
  That maybe if this was created 64 bit format it would read ok. Seems unlikely but this bug is kind of strange. .. seems unlikely.

• Another possibility. Perhaps we are mixing and matching 32 and 64 bit calls to minizip? The total_out_64 and total_out seem to have got out of sync (these are the file pointers I believe).

I wonder whether this bug also has been seen in 4.x.

Also intriguing:

• The return type of zuntell seems to be signed (32 bit?)
• The return type of zuntell64 seems to be unsigned (64 bit)

Zip within zip

Ah, just read you are reading a zip within a zip, this is more suggestive of a shared variable in minizip that is breaking the operation. It may be an idea to try an MRP in 4.x, because I suspect it may occur there too, and we could get more eyes on it that way.

It is possible this is not officially supported (by minizip), it might be required to decompress the first zip to raw memory, then decompress the second zip from the unzipped version. But if this is the case it's amazing it worked before. 🤔

UPDATE: It seems to fail reading in the first zip with _load_resource_pack() rather than the 2nd.

[lawnjelly]

This seems quite tricky to pin down and spending longer than I have this morning:

I think for now if @Kersoph was able to try creating an MRP in 4.x that would be super useful.

• If it doesn't occur in 4.x then we know there are related changes to find.
• If it does, we can get more contributors looking at it.

Also, how much of a problem is this for you guys?
We can also consider reverting until we track this down (given that it hadn't caused a problem I'm guessing for Godot users previous to 3.6)?

[kiroxas]

This seems quite tricky to pin down and spending longer than I have this morning:

I think for now if @Kersoph was able to try creating an MRP in 4.x that would be super useful.

• If it doesn't occur in 4.x then we know there are related changes to find.
• If it does, we can get more contributors looking at it.

Also, how much of a problem is this for you guys? We can also consider reverting until we track this down (given that it hadn't caused a problem I'm guessing for Godot users previous to 3.6)?

Just tested on 4.5 master and it does occur there too and reverting this commit makes it work again, so somehow something is being done differently between unztell and unztell64

Code for 4.5

extends Control

var mod_pack_path = "res://MyMod.zip"
var mod_internal_data_path = "res://MyMod/res.zip"
var mod_texture_path = "res://Icons/OK.png"


func _ready():
	var mod_pack_path_loaded_successfully = ProjectSettings.load_resource_pack("res://MyMod.zip", true)
	var mod_internal_data_path_loaded_successfully = ProjectSettings.load_resource_pack(mod_internal_data_path, true)
	
	var file = FileAccess.open(mod_texture_path, FileAccess.READ)
	var file_buffer = file.get_buffer(file.get_length())
	file.close()
	var mod_image = Image.new()
	mod_image.load_png_from_buffer(file_buffer)
	var texture = ImageTexture.new()
	texture.create_from_image(mod_image) #,0
	get_node("TextureRect").texture = texture
	
	var mod_direct_data_path_loaded_successfully = ProjectSettings.load_resource_pack("res://res.zip", true)

[lawnjelly]

I'll mark this as 4.x to get more traction, thanks for testing.