daniu54 / earie-treasures

A 2d top-down loot-em-up with horror and stealth elements
0 stars 0 forks source link

Verify that scene load issue is gone #20

Closed Abb4 closed 1 year ago

Abb4 commented 1 year ago

In #5 an issue with loading checked-out godot scenes was reported. Apparently NavigateTowards2dNodes.cs script was not included correctly. We suspect malformed godot scene files. Re-including the script into the nav_mesh_2d_seeker.tscn godot scene seemed to fix the issue.

On my pc I was able to start the project without issues.

If the issue persists after future pull requests it should be investigated further

Abb4 commented 1 year ago

@softwaredev101 I just merged a tiny pr: #21

Could you verify that the scene loading issue is gone with current main? Before we get the issue with massive prs?

I have looked into the scene files and I just can not figure out why you get the issue. If I remove the script, save the scene and reassign it, and save the scene, I get no changes in git. This means that the asset files (here specifically NavigateTowards2dNodes), whenever they are assigned/imported, always get the same id.

I suspect two causes for this behavior:

  1. (script-) files cache their ids somewhere in the project, outside of scene files
  2. Ids are not fully randomly generated

As for 1.: I could not find the file where these Ids are cached before being used in scene files. Therefore I assume 2. As for 2.: I could imagine that godot hashes resource files and generates the Id from that. This means that including the same resource two times will always result in the same id.

I believe this was not always the case as evidenced by https://github.com/godotengine/godot-proposals/issues/471 In godot 4 id generation was changed to favor git friendly formats: https://github.com/godotengine/godot/pull/50676

I tried to understand the changed code but my brain is too smooth for cpp.

I propose we investigate further if the issue arises again. If it does not we can close this with clear conscience

softwaredev101 commented 1 year ago

@Abb4 Thanks for looking into this problem Unfortunately, the crashes persist. Regarding your statements about Git, it is normal. If you detach a script from the corresponding node and reattach it, no changes have been made since you've been going back to the beginning. Regarding the C++ code I can extract the following insights:

From my perspective changing the Script name to another name than the NavigateTowards2dNodes solved the problem after crashing once again (yeah, I know it's likely not ideal). Keep in mind that renaming scripts may break the corresponding nodes since we need a 1:1 mapping from class name to script file name

Abb4 commented 1 year ago

God damn it, It does cache 🙈

I forgot about .gitignore.

In .godot folder, which is excluded from git there seem to be changes after fixing the packed scene. Now I need to find the changes, which cause the crashing and include them in the commit log to avoid packed scene issues.

Abb4 commented 1 year ago

One of the changes in https://github.com/Abb4/earie-treasures/commit/592ebabcbac5ffb595d774bc7bcc964f8a1264ac should be the culprit.

Abb4 commented 1 year ago

I still need to find the file inside .godot that we need to keep.

@softwaredev101 If we merge as it currently is, you get a bunch of my internal editor settings every time we sync work (e.g. currently selected scene)