godot icon indicating copy to clipboard operation
godot copied to clipboard

Random Custom Resource Corruption When Upgrading to v4.4.beta3

Open yongan-game opened this issue 10 months ago • 10 comments

Tested versions

Reproducible in: v4.4.beta3, v4.4beta4 Not reproducible in: v4.4.beta2 and earlier

System information

Godot v4.4.beta3 - Windows 11 (build 22631) - Multi-window, 2 monitors - OpenGL 3 (Compatibility) - NVIDIA GeForce GTX 1650 (NVIDIA; 32.0.15.6614) - 11th Gen Intel(R) Core(TM) i5-11400H @ 2.70GHz (12 threads)

Issue description

One of our custom resource types (specifically EmployeeJob, as seen in the MRP) that functioned in previous versions breaks with v4.4.beta3; specifically, for any old resources of this type, it seems like Godot completely disregards the custom script and treats it as a plain Resource.


Take the following trivial scene:

extends Node

@export var jobs: Array[EmployeeJob]

func _ready():
	for job: EmployeeJob in jobs:
		print(job.id)

Running this scene with any old resources of this type, we get the following in v4.4.beta3: Invalid access to property or key 'id' on a base object of type 'Resource'.

Image

When looking in the inspector, the EmployeeJob resource is missing all of its members: Image

These members aren't missing in the inspector when opening the resource through the FileSystem, even on v4.4.beta3.

Running this same scene in v4.4.beta2 has no issues at all. In fact, the corruption seems to be exclusive to v4.4.beta3, in that it works in beta 2 without any changes to any files at all.

Image

Additionally, all the class members show up properly in the inspector. Image

Looking at the UIDs of all scripts/resources involved, as well as global_script_class_cache.cfg, nothing seems out of the ordinary between the two versions. Also, none of our other custom Resource types have been affected, it is only this one.

An excerpt from global_script_class_cache.cfg:

{
"base": &"GameIDResource",
"class": &"EmployeeJob",
"icon": "",
"is_abstract": false,
"is_tool": true,
"language": &"GDScript",
"path": "res://src/resources/jobs/job.gd"
}

This bug seems to only occur with EmployeeJob resources that were in the project before upgrading to v4.4.beta3. Creating a new EmployeeJob resource and testing it with the same scene works fine, even in v4.4.beta3. Comparing the two .tres files, I don't see any reason why this would be the case, however. (Note that the following resources are from our main project, not the MRP)

job_brewer.tres: Old resource, does not work

[gd_resource type="Resource" script_class="EmployeeJob" load_steps=3 format=3 uid="uid://dlfahaeajxmcf"]

[ext_resource type="Texture2D" uid="uid://cwyjvuk6yxkpg" path="res://assets/img/ui/icons/jobs/job_icon_brewer.png" id="1_ou54x"]
[ext_resource type="Script" uid="uid://bctjqgajd245e" path="res://src/resources/jobs/job.gd" id="2_612de"]

[resource]
resource_name = "job_brewer"
script = ExtResource("2_612de")
exp_cutoffs = Array[int]([500, 1000, 2000, 3500, 5000, 10000])
salary_levels = Array[int]([5, 10, 15, 20, 25, 30])
job_speed_levels = Array[float]([2.0, 1.5, 1.1, 0.9, 0.75, 0.5])
job_icon = ExtResource("1_ou54x")
stat_change_amounts = {
0: 0.05,
1: 0.1,
2: 0.2
}
exp_per_day = 15
stat_change_stress = 0
stat_change_spirit = 1
stat_change_motivation = 1
_update = false
metadata/_custom_type_script = ExtResource("2_612de")

job_test.tres: Newly created resource, does work

[gd_resource type="Resource" script_class="EmployeeJob" load_steps=3 format=3 uid="uid://drarbwqpl8fea"]

[ext_resource type="Texture2D" uid="uid://bt4g71td57t5q" path="res://assets/img/ui/common/ui-grabber.png" id="1_pumyt"]
[ext_resource type="Script" uid="uid://bctjqgajd245e" path="res://src/resources/jobs/job.gd" id="2_xs1mv"]

[resource]
resource_name = "job_test"
script = ExtResource("2_xs1mv")
exp_cutoffs = Array[int]([500, 1000, 2000, 3500, 5000, 10000])
salary_levels = Array[int]([5, 10, 15, 20, 25, 30])
job_speed_levels = Array[float]([2.0, 1.5, 1.1, 0.9, 0.75, 0.5])
job_icon = ExtResource("1_pumyt")
stat_change_amounts = {
0: 0.05,
1: 0.1,
2: 0.2
}
exp_per_day = 15
stat_change_stress = 1
stat_change_spirit = 1
stat_change_motivation = 1
_update = false
metadata/_custom_type_script = ExtResource("2_xs1mv")

Steps to reproduce

Unfortunately, this MRP should work just fine for anyone testing; this error specifically occurs when upgrading our project to v4.4.beta3. Additionally, the MRP was created in v4.4.beta2 and then tested in v.4.4.beta3 and the error still doesn't occur. The MRP is included in any case to potentially help diagnose the issue. The classes included are exactly as they are in our own code base, save for some removed code in util.gd that is not used anywhere in the MRP.

  • Open MRP
  • Run main.tscn. While it should work for anyone testing this, this same code causes the following error in our main project: Invalid access to property or key 'id' on a base object of type 'Resource'.

Minimal reproduction project (MRP)

test_4.4b3_resource_regression.zip

yongan-game avatar Feb 11 '25 00:02 yongan-game

Probably is the same problem as #102557 and #102568

matheusmdx avatar Feb 11 '25 10:02 matheusmdx

It's possible, but I tried deleting the .godot folder, which worked for those other two issues, and the problem persists.

yongan-game avatar Feb 11 '25 15:02 yongan-game

Could you test the Windows CI build from #102636 and see if it solves your issue? See https://docs.godotengine.org/en/stable/contributing/workflow/testing_pull_requests.html

akien-mga avatar Feb 11 '25 21:02 akien-mga

No luck unfortunately; I tried that build with both my current .godot folder, as well as after deleting .godot, and I still get the same errors.

yongan-game avatar Feb 11 '25 21:02 yongan-game

If you're able to compile Godot from source, bisecting the regression would be quite helpful to pinpoint which commit introduced it.

Between 4.4.beta2 and 4.4.beta3, that would be bisecting between a013481b0 (good) and 06acfccf8 (bad).

akien-mga avatar Feb 11 '25 22:02 akien-mga

According to my bisection, daa074881b7d2e587bab814c0523d5d26edbe802 is the culprit; I can't say I know enough to diagnose this, but thankfully the commit doesn't seem to change that much so it should be relatively easy to pinpoint (hopefully at least 👀)

yongan-game avatar Feb 12 '25 00:02 yongan-game

According to my bisection, daa0748 is the culprit; I can't say I know enough to diagnose this, but thankfully the commit doesn't seem to change that much so it should be relatively easy to pinpoint (hopefully at least 👀)

That's tricky, because that commit (#102429) is a revert of a change also made in 4.4 (#96499), which caused regressions.

So if you don't have the bug in 4.3 (which doesn't have #96499 which was later reverted), it means the bug is probably caused by another change, but was hidden between the merge of #96499 and its revert.

#96499 was merged for 4.4-dev4. Do you reproduce the bug in that snapshot, and previous 4.4 dev snapshots?

akien-mga avatar Feb 14 '25 09:02 akien-mga

Hi, apologies for the late response. I don't have any issues at all with 4.4-dev4 or any earlier 4.4 dev versions. I could try testing 4.3 if that'll help, but that'll be harder since our project relies on some stuff introduced in 4.4.

I should also add that the issue persists in the recently released 4.4.beta4 too.

yongan-game avatar Feb 18 '25 21:02 yongan-game

One of our other team members just tested on one of her machines, and the issue exists there too, further leading me to think it has nothing to do with the .godot cache (unless we both have the same issue on different machines).

Her system info: Godot v4.4.beta4 - macOS Catalina (10.15.7) - Multi-window, 1 monitor - OpenGL 3 (Compatibility) - Intel Iris OpenGL Engine - Intel(R) Core(TM) i5-4258U CPU @ 2.40GHz (4 threads)

yongan-game avatar Feb 19 '25 22:02 yongan-game

I've managed to fix my issue through some workarounds of which I'm not entirely sure how they work; the solution makes me think the issue wasn't any kind of corruption, but some odd change in how Godot handled circular dependencies in 4.4.beta2 vs. 4.4.beta3 onwards. I'll detail the solution below, in case it helps anyone, as well as give my insight as to what might have caused this regression, maybe. Apologies if this comment ends up bloating up the comment chain


First, I need to mention another class that's part of our project, job_data.gd, which is loaded as an Autoload for our project (using the global name JobData). I didn't include it in the MRP because I assumed it was irrelevant to the bug, but it turned out not to be. Some relevant excerpts are shown below.

job_data.gd

extends Node

@export var NONE: EmployeeJob
@export var KITCHEN: EmployeeJob
@export var SERVER: EmployeeJob
@export var JANITOR: EmployeeJob
@export var DELIVERY: EmployeeJob
@export var GUARD: EmployeeJob
@export var DISHWASHER: EmployeeJob
@export var ENTERTAINER: EmployeeJob
@export var HOST: EmployeeJob
@export var TEA_MASTER: EmployeeJob
@export var BREWER: EmployeeJob

# Don't worry about the actual values here, but this particular const is critical to my fix
const ESSENTIAL_JOBS_BY_ESTABLISHMENT := {
	Global.EstCategory.RESTAURANT: [ "job_kitchen", [ "job_server", "job_host", ] ],
	Global.EstCategory.TEASHOP: [ "job_kitchen", "job_server", ],
	Global.EstCategory.PUB: [ "job_kitchen", "job_server", ],
	Global.EstCategory.FASTFOOD: [ "job_kitchen", "job_host", ],
}

# Notice how the function is currently not static
func get_job_by_id(id: String) -> EmployeeJob:
	for job in Jobs.values():
		if job.job_id == id: return job
	return null

My fix was as follows:

  1. Remove JobData as an autoload
  2. Turn JobData into a globally accessible class_name, converting all functions and variables into static:
class_name JobData
extends Node
static func get_job_by_id(id: String) -> EmployeeJob:
# etc.
  1. Move the ESSENTIAL_JOBS_BY_ESTABLISHMENT const from JobData into EmployeeJob, and update all references to the old value accordingly; of particular note, this changes the following line in job.gd ( I should note that this line is commented out in my MRP, which is a big slip up on my end, I apologize):
var is_essential: bool :
    get:
        if Engine.is_editor_hint(): return false
            # Old line, commented out
            #for entry in JobData.ESSENTIAL_JOBS_BY_ESTABLISHMENT[GameVars.establishment_category]:
            # New, fixed line
            for entry in ESSENTIAL_JOBS_BY_ESTABLISHMENT[GameVars.establishment_category]:
                if not entry: continue
                if entry is String and id == entry:
                    return true
                elif entry is Array and id in entry:
                    return true
            return false
  1. Project now works completely fine, from 4.4.beta3 up to 4.4.rc1; no changes were needed to any previous EmployeeJob resources

Changing JobData from an autoload to a globally named script was essential to the fix; I tried leaving it as an autoload, and the same glitch kept occuring.


I don't know what about daa074881b7d2e587bab814c0523d5d26edbe802, or any commits between 4.4.beta2 and 4.4.beta3, might have caused this issue, but the fact that removing a single JobData reference from EmployeeJob fixed it leads me to believe it was related to a cyclic dependency somehow. In this new design, EmployeeJob never references JobData, and it now seems to work.

Part of what makes me think it's related to circular dependencies is the following errors I got when I only did steps 1 and 2:

Image

This error seems quite similar to #99333. I don't know why changing JobData to a globally named script caused the errors to change from the ones I was getting in the original post to the ones shown above, however.

I'm not entirely happy marking this as closed yet, because the solution is very arcane (I still don't know why JobData can't be an autoload for this to work), and the fact stands that something that worked in old Godot versions suddenly doesn't work in new ones, but given that if it's a circular dependency it's completely my own fault, and that I found a way to resolve my issue in any case, please feel free to close this issue at your discretion.

yongan-game avatar Feb 23 '25 05:02 yongan-game

Thanks for the extensive testing and writeup! I'm de-prioritizing this from a 4.4 release blocker, but it does sound like there's more to investigate. Cyclic dependencies aren't well supported and so the way such code behaves is a bit hit and miss, but it's weird that there is a difference between 4.3 and 4.4 caused by a revert of a 4.4 exclusive change.

There must have been another change between #96499 and #102429 that triggered this issue, but which was hidden by the effect of #96499.

akien-mga avatar Feb 25 '25 16:02 akien-mga

I wonder if https://github.com/godotengine/godot/pull/97302 would help solve this.

akien-mga avatar Mar 01 '25 08:03 akien-mga