Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix id encoded with incorrect signedness. #662

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix id encoded with incorrect signedness. #662

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5a89c5f
Fix id being decoded as an signed integer instead of an unsigned inte…
zarstensen Jun 5, 2024
919fc7f
Fix properties of inspected objects not being added to inspection.
zarstensen Jun 5, 2024
82507ce
Merge branch 'master' into fix_id_sign
DaelonSuzuka Jun 24, 2024
9180194
Fix bad merge
DaelonSuzuka Jun 24, 2024
12f211e
Fix objects being inspected even when not viewed in variables view, l…
zarstensen Jun 25, 2024
a164f84
Merge branch 'master' of https://github.com/godotengine/godot-vscode-…
zarstensen Aug 2, 2024
e88ef7f
Merge 'godotengine-master' into fix_id_sign
zarstensen Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/debugger/godot3/debug_session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ export class GodotDebugSession extends LoggingDebugSession {
this.append_variable(va);
});

this.add_to_inspections();
this.all_scopes.forEach(scope => this.add_to_inspections(scope?.sub_values));

if (this.ongoing_inspections.length === 0) {
this.previous_inspections = [];
Expand Down Expand Up @@ -421,8 +421,8 @@ export class GodotDebugSession extends LoggingDebugSession {
}
}

private add_to_inspections() {
this.all_scopes.forEach((va) => {
public add_to_inspections(variables: GodotVariable[]) {
variables?.forEach((va) => {
if (va && va.value instanceof ObjectId) {
if (
!this.ongoing_inspections.includes(va.value.id) &&
Expand Down
34 changes: 0 additions & 34 deletions src/debugger/godot3/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,40 +34,6 @@ export function is_variable_built_in_type(va: GodotVariable) {
return ["number", "bigint", "boolean", "string"].some(x => x == type);
}

export function build_sub_values(va: GodotVariable) {
const value = va.value;

let subValues: GodotVariable[] = undefined;

if (value && Array.isArray(value)) {
subValues = value.map((va, i) => {
return { name: `${i}`, value: va } as GodotVariable;
});
} else if (value instanceof Map) {
subValues = Array.from(value.keys()).map((va) => {
if (typeof va["stringify_value"] === "function") {
return {
name: `${va.type_name()}${va.stringify_value()}`,
value: value.get(va),
} as GodotVariable;
} else {
return {
name: `${va}`,
value: value.get(va),
} as GodotVariable;
}
});
} else if (value && typeof value["sub_values"] === "function") {
subValues = value.sub_values().map((sva) => {
return { name: sva.name, value: sva.value } as GodotVariable;
});
}

va.sub_values = subValues;

subValues?.forEach(build_sub_values);
}

export function parse_variable(va: GodotVariable, i?: number) {
const value = va.value;
let rendered_value = "";
Expand Down
77 changes: 71 additions & 6 deletions src/debugger/godot3/server_controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { debug, window } from "vscode";
import { StoppedEvent, TerminatedEvent } from "@vscode/debugadapter";
import { VariantEncoder } from "./variables/variant_encoder";
import { VariantDecoder } from "./variables/variant_decoder";
import { RawObject } from "./variables/variants";
import { GodotStackFrame, GodotStackVars } from "../debug_runtime";
import { ObjectId, RawObject } from "./variables/variants";
import { GodotStackFrame, GodotVariable, GodotStackVars } from "../debug_runtime";
import { GodotDebugSession } from "./debug_session";
import { parse_next_scene_node, split_buffers, build_sub_values } from "./helpers";
import { parse_next_scene_node, split_buffers } from "./helpers";
import { get_configuration, get_free_port, createLogger, verify_godot_version, get_project_version, clean_godot_path } from "../../utils";
import { prompt_for_godot_executable } from "../../utils/prompts";
import { subProcess, killSubProcesses } from "../../utils/subspawn";
Expand Down Expand Up @@ -375,16 +375,21 @@ export class ServerController {
break;
}
case "message:inspect_object": {
const id = BigInt(command.parameters[0]);
let id = BigInt(command.parameters[0]);
const className: string = command.parameters[1];
const properties: any[] = command.parameters[2];

// message:inspect_object returns the id as an unsigned 64 bit integer, but it is decoded as a signed 64 bit integer,
// thus we need to convert it to its equivalent unsigned value here.
if(id < 0)
id = id + BigInt(2) ** BigInt(64);

const rawObject = new RawObject(className);
properties.forEach((prop) => {
rawObject.set(prop[0], prop[5]);
});
const inspectedVariable = { name: "", value: rawObject };
build_sub_values(inspectedVariable);
this.build_sub_values(inspectedVariable);
if (this.session.inspect_callbacks.has(BigInt(id))) {
this.session.inspect_callbacks.get(BigInt(id))(
inspectedVariable.name,
Expand Down Expand Up @@ -543,8 +548,68 @@ export class ServerController {
stackVars.globals.push({ name: parameters[i++], value: parameters[i++] });
}

stackVars.forEach(item => build_sub_values(item));
stackVars.forEach(item => this.build_sub_values(item));

this.session.set_scopes(stackVars);
}

private build_sub_values(va: GodotVariable) {
const value = va.value;

let subValues: GodotVariable[] = undefined;

if (value && Array.isArray(value)) {
subValues = value.map((va, i) => {
const gd_var = {
name: `${i}`,
value: va,
} as GodotVariable;

if(gd_var.value instanceof ObjectId)
this.session.add_to_inspections([ gd_var ]);

return gd_var;
});
} else if (value instanceof Map) {
subValues = Array.from(value.keys()).map((va) => {
if (typeof va["stringify_value"] === "function") {
const gd_var = {
name: `${va.type_name()}${va.stringify_value()}`,
value: value.get(va),
} as GodotVariable;

if(gd_var.value instanceof ObjectId)
this.session.add_to_inspections([ gd_var ]);

return gd_var;
} else {
const gd_var = {
name: `${va}`,
value: value.get(va),
} as GodotVariable;

if(gd_var.value instanceof ObjectId)
this.session.add_to_inspections([ gd_var ]);

return gd_var;
}
});
} else if (value && typeof value["sub_values"] === "function") {
subValues = value.sub_values().map((sva) => {
const gd_var = {
name: sva.name,
value: sva.value,
} as GodotVariable;

if(gd_var.value instanceof ObjectId)
this.session.add_to_inspections([ gd_var ]);

return gd_var;
});
}

va.sub_values = subValues;

subValues?.forEach(this.build_sub_values.bind(this));
}
}
6 changes: 3 additions & 3 deletions src/debugger/godot4/debug_session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ export class GodotDebugSession extends LoggingDebugSession {
this.append_variable(va);
});

this.add_to_inspections();
this.all_scopes.forEach(scope => this.add_to_inspections(scope?.sub_values));

if (this.ongoing_inspections.length === 0) {
this.previous_inspections = [];
Expand Down Expand Up @@ -421,8 +421,8 @@ export class GodotDebugSession extends LoggingDebugSession {
}
}

private add_to_inspections() {
this.all_scopes.forEach((va) => {
public add_to_inspections(variables: GodotVariable[]) {
variables?.forEach((va) => {
if (va && va.value instanceof ObjectId) {
if (
!this.ongoing_inspections.includes(va.value.id) &&
Expand Down
34 changes: 0 additions & 34 deletions src/debugger/godot4/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,40 +36,6 @@ export function is_variable_built_in_type(va: GodotVariable) {
return ["number", "bigint", "boolean", "string"].some(x => x == type);
}

export function build_sub_values(va: GodotVariable) {
const value = va.value;

let subValues: GodotVariable[] = undefined;

if (value && Array.isArray(value)) {
subValues = value.map((va, i) => {
return { name: `${i}`, value: va } as GodotVariable;
});
} else if (value instanceof Map) {
subValues = Array.from(value.keys()).map((va) => {
if (typeof va["stringify_value"] === "function") {
return {
name: `${va.type_name()}${va.stringify_value()}`,
value: value.get(va),
} as GodotVariable;
} else {
return {
name: `${va}`,
value: value.get(va),
} as GodotVariable;
}
});
} else if (value && typeof value["sub_values"] === "function") {
subValues = value.sub_values().map((sva) => {
return { name: sva.name, value: sva.value } as GodotVariable;
});
}

va.sub_values = subValues;

subValues?.forEach(build_sub_values);
}

export function parse_variable(va: GodotVariable, i?: number) {
const value = va.value;
let rendered_value = "";
Expand Down
83 changes: 76 additions & 7 deletions src/debugger/godot4/server_controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { debug, window } from "vscode";
import { StoppedEvent, TerminatedEvent } from "@vscode/debugadapter";
import { VariantEncoder } from "./variables/variant_encoder";
import { VariantDecoder } from "./variables/variant_decoder";
import { RawObject } from "./variables/variants";
import { ObjectId, RawObject } from "./variables/variants";
import { GodotStackFrame, GodotVariable, GodotStackVars } from "../debug_runtime";
import { GodotDebugSession } from "./debug_session";
import { parse_next_scene_node, split_buffers, build_sub_values } from "./helpers";
import { parse_next_scene_node, split_buffers } from "./helpers";
import { get_configuration, get_free_port, createLogger, verify_godot_version, get_project_version, clean_godot_path } from "../../utils";
import { prompt_for_godot_executable } from "../../utils/prompts";
import { subProcess, killSubProcesses } from "../../utils/subspawn";
Expand Down Expand Up @@ -374,16 +374,21 @@ export class ServerController {
break;
}
case "scene:inspect_object": {
const id = BigInt(command.parameters[0]);
let id = BigInt(command.parameters[0]);
const className: string = command.parameters[1];
const properties: any[] = command.parameters[2];

// scene:inspect_object returns the id as an unsigned 64 bit integer, but it is decoded as a signed 64 bit integer,
// thus we need to convert it to its equivalent unsigned value here.
if(id < 0)
id = id + BigInt(2) ** BigInt(64);

const rawObject = new RawObject(className);
properties.forEach((prop) => {
rawObject.set(prop[0], prop[5]);
});
const inspectedVariable = { name: "", value: rawObject };
build_sub_values(inspectedVariable);
this.build_sub_values(inspectedVariable);
if (this.session.inspect_callbacks.has(BigInt(id))) {
this.session.inspect_callbacks.get(BigInt(id))(
inspectedVariable.name,
Expand Down Expand Up @@ -411,8 +416,12 @@ export class ServerController {
break;
}
case "stack_frame_vars": {
this.partialStackVars.reset(command.parameters[0]);
this.session.set_scopes(this.partialStackVars);
const stack_var_count = command.parameters[0];
this.partialStackVars.reset(stack_var_count);

if(stack_var_count <= 0)
this.session.set_scopes(this.partialStackVars);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. image

#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.


break;
}
case "stack_frame_var": {
Expand Down Expand Up @@ -551,7 +560,7 @@ export class ServerController {
}

const variable: GodotVariable = { name, value, type };
build_sub_values(variable);
this.build_sub_values(variable);

const scopeName = ["locals", "members", "globals"][scope];
this.partialStackVars[scopeName].push(variable);
Expand All @@ -561,4 +570,64 @@ export class ServerController {
this.session.set_scopes(this.partialStackVars);
}
}

private build_sub_values(va: GodotVariable) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const value = va.value;

let subValues: GodotVariable[] = undefined;

if (value && Array.isArray(value)) {
subValues = value.map((va, i) => {
const gd_var = {
name: `${i}`,
value: va,
} as GodotVariable;

if(gd_var.value instanceof ObjectId)
this.session.add_to_inspections([ gd_var ]);

return gd_var;
});
} else if (value instanceof Map) {
subValues = Array.from(value.keys()).map((va) => {
if (typeof va["stringify_value"] === "function") {
const gd_var = {
name: `${va.type_name()}${va.stringify_value()}`,
value: value.get(va),
} as GodotVariable;

if(gd_var.value instanceof ObjectId)
this.session.add_to_inspections([ gd_var ]);

return gd_var;
} else {
const gd_var = {
name: `${va}`,
value: value.get(va),
} as GodotVariable;

if(gd_var.value instanceof ObjectId)
this.session.add_to_inspections([ gd_var ]);

return gd_var;
}
});
} else if (value && typeof value["sub_values"] === "function") {
subValues = value.sub_values().map((sva) => {
const gd_var = {
name: sva.name,
value: sva.value,
} as GodotVariable;

if(gd_var.value instanceof ObjectId)
this.session.add_to_inspections([ gd_var ]);

return gd_var;
});
}

va.sub_values = subValues;

subValues?.forEach(this.build_sub_values.bind(this));
}
}