godotengine / godot-vscode-plugin

Godot development tools for VSCode
MIT License
1.47k stars 148 forks source link

Fix id encoded with incorrect signedness. #662

Open zarstensen opened 4 weeks ago

zarstensen commented 4 weeks ago

This PR fixes #660.

Issue:

Id's sent via. the scene:inspect_object and message:inspect_object commands are encoded as 64 bit unsigned integers, but they are decoded as 64 bit signed integers. This leads to the debugger not finding the resources properties and methods due to mismatching id's, if the resources id (or any other object with an id) is greater than the maximum value of an signed 64 bit integer. This seemingly happens because the VariantDecoder class is unable to see if the int type is signed or unsigned, so it defaults to it being signed.

Resolution:

Convert the signed id value to its unsigned equivalent when handling scene:inspect_object or message:inspect_object commands.

zarstensen commented 4 weeks ago

Upon further research it seems this only partly fixes #660, as it does not work if the resource is present in a property of another object, then it is again only the id which is shown. image

This also happens for Object types and seemingly has something to do with the fact that properties and methods are not retrieved for objects which are present in properties of other objects?

DaelonSuzuka commented 4 weeks ago

Awesome, thank you for documenting all this. I didn't have time to investigate every known debugger issue before releasing v2.0, but this is a great lead and I'll probably be able to dig into it next week.

zarstensen commented 3 weeks ago

I'm glad i could help!

I think i also fixed the other issue mentioned here in the latest commit to this PR.

Just as a quick reference here is a before and after of this fix on my machine:

Before Fix:

First time debugging script. image

Restarting debugging, with custom_object expanded from previous debug session. image

After Fix:

Previous scenarios both lead to this result. image

Here is the source code used to produce the above images:

root_node.gd

extends Node

func _ready() -> void:

    var custom_object: CustomObject = CustomObject.new()

    custom_object.object_variable = CustomResource.new(1234)
    custom_object.object_variable_in_array = [ CustomResource.new(9999) ]
    custom_object.object_variable_in_dict = { 'key': CustomResource.new(-1) }

    breakpoint

custom_resource.gd

class_name CustomResource
extends Resource

var resource_variable

func _init(val: int):
    resource_variable = val

custom_object.gd

class_name CustomObject
extends Object

var object_variable: CustomResource

var object_variable_in_array: Array[CustomResource]

var object_variable_in_dict: Dictionary
DaelonSuzuka commented 1 week ago

I hate the merge editor so much.

DaelonSuzuka commented 1 week ago

In an actual project (aka more than 5 objects), the first breakpoint I hit just spawns infinite communications:

image

This has been going for several minutes now.

I'm suddenly reminded that when I was working on the debugger, I had to intentionally not send all the inspect_object requests that I had object ids for because the volume of communications was just too massive. Unfortunately I didn't discover a unifying pattern AND I didn't properly document this in the code.

zarstensen commented 1 week ago

I guess the projects i used to test were just too small to catch this performance issue.

Anyways, i believe the most recent commit fixes this by only inspecting objects when they are explicitly requested in the variablesRequest method.

I have tried to test the performance of it by adding an array of custom objects with length 10000, to simulate a project with a large quantity of objects.

New Fix:

Took around 12 seconds to finish. image image small amount of inspect requests, only called on objects visible in the variables tree.

primary bottleneck was looping through the GodotDebugSession.all_scopes 10000 times via. the find method (Source code location)

Old Fix:

Took around 1 min 24 sec to finish. image image way too many inspect requests compared to new fix.

DaelonSuzuka commented 1 week ago

Can you test it in Godot 3, also? It looks like it's working fine in Godot 4 now, but I still got an inspection cascade in a Godot 3 project.

zarstensen commented 1 week ago

Hm, thats wierd.

My test does not have any issues even if converted to a Godot 3 project:

image

All output from debug start to breakpoint hit in godot 3 project.

I do not really have any large godot 3 projects laying around to further test, so is there a chance you could send a project which has this issue?