Fix id encoded with incorrect signedness.

[zarstensen]

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]

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.

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]

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]

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.

Restarting debugging, with custom_object expanded from previous debug session.

After Fix:

Previous scenarios both lead to this result.

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

[zarstensen]

if set_scopes is called here every time the stack_frame_vars command is handled, it leads to the issue seen in the second image in the following comment:

Restarting debugging, with custom_object expanded from previous debug session.

#662 (comment)

This if statement makes sure GodotDebugSession.ongoing_inspections is not of length 0 (if Objects are present in the current stack frame) the first time set_scopes is called, thus preventing GodotDebugSession.got_scope.notify being called too early.

[zarstensen]

build_sub_values was moved from helpers.ts to server_controller.ts and was converted to a method of ServerController, since only this object used it anyways and a ServerController instance was required in the new version.

Additionally, any time a GodotVariable is constructed, GodotDebugSession.add_to_inspections is called on it, ensuring all sub values of a variable gets inspected as well.

[DaelonSuzuka]

I hate the merge editor so much.

[DaelonSuzuka]

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

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]

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.


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.


way too many inspect requests compared to new fix.

[DaelonSuzuka]

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]

Hm, thats wierd.

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

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?