Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 10 additions & 7 deletions src/app/services/data/data.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ export class DataService implements OnDestroy {

private destroyed = new Subject<void>();

// Counter to force D3 data rebinding when loading new DTO (not for undo/redo)
// Fixes issue #851: prevents stale object references in D3 callbacks
private netzgrafikLoadCounter = 0;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This load counter is the most important new "thing" I have introduced. The load counter ensures that during runtime each loadNetzgrafikDto will get a new LC and thus the rendering gets updated through ...ViewObjects.key --> change


private readonly netzgrafikLoadedInfoSubject = new BehaviorSubject<NetzgrafikLoadedInfo>(
new NetzgrafikLoadedInfo(true, false),
);
Expand Down Expand Up @@ -72,7 +76,12 @@ export class DataService implements OnDestroy {
this.destroyed.complete();
}

getNetzgrafikLoadCounter(): number {
return this.netzgrafikLoadCounter;
}

loadNetzgrafikDto(netzgrafikDto: NetzgrafikDto, preview = false) {
this.netzgrafikLoadCounter++;
this.netzgrafikLoadedInfoSubject.next(new NetzgrafikLoadedInfo(true, preview));

DataMigration.migrateNetzgrafikDto(netzgrafikDto);
Expand Down Expand Up @@ -151,13 +160,7 @@ export class DataService implements OnDestroy {
this.trainrunSectionService.initializeTrainrunSectionRouting();
this.nodeService.validateAllConnections();
this.trainrunService.propagateInitialConsecutiveTimes();
this.nodeService.nodesUpdated();
this.nodeService.transitionsUpdated();
this.nodeService.connectionsUpdated();
this.trainrunSectionService.trainrunSectionsUpdated();
this.noteService.notesUpdated();
this.labelService.labelUpdated();
this.labelGroupService.labelGroupUpdated();
this.triggerViewUpdate();
Copy link
Copy Markdown
Contributor Author

@aiAdrian aiAdrian Feb 26, 2026

Choose a reason for hiding this comment

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

It found by hazard this duplicated code. While testing to "clear" all object during load which could overcome the problem. But this leads to more complicated things to do. This was the reason i stop changed the loading stuff.

I think we have just to ensure when then callbacks gets correctly handled. Once we face a comparison by object in the code where we compare "rendering system stored / cached objects" with ...Service stored we should focus on id based comparision not on objects. Thus we haven't to change the loading stuff. This problem was located at wrong comparision logic.


this.netzgrafikColoringService.generateGlobalStyles(
this.getTrainrunCategories(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export class ConnectionsViewObject {
displayConnectionPin2: boolean,
): string {
let key =
"LC" +
editorView.getNetzgrafikLoadCounter() +
"#" +
connection.getId() +
"@" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ describe("3d.Utils.tests", () => {
levelOfDetailService,
undefined,
positionTransformationService,
dataService,
);

new EditorView(
Expand Down
13 changes: 7 additions & 6 deletions src/app/view/editor-main-view/data-views/data.view.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ describe("Editor-DataView", () => {
levelOfDetailService,
undefined,
positionTransformationService,
dataService,
);

new EditorView(
Expand Down Expand Up @@ -188,7 +189,7 @@ describe("Editor-DataView", () => {

const cvo = new ConnectionsViewObject(editorView, con, node, false, false);
expect(cvo.key).toBe(
"#2@false_15_9_false_(832,144)_(774.4,144)_(793.6,144)_(736,144)_false_false_false_0_false(832,144)(774.4,144)(793.6,144)(736,144)",
"LC1#2@false_15_9_false_(832,144)_(774.4,144)_(793.6,144)_(736,144)_false_false_false_0_false(832,144)(774.4,144)(793.6,144)(736,144)",
);
});

Expand All @@ -197,18 +198,18 @@ describe("Editor-DataView", () => {
const node = nodeService.getNodeFromId(2);

const cvo1 = new NodeViewObject(editorView, node, false);
expect(cvo1.key).toBe("#2@736_64_ZUE_6_5_96_124_false_2_false_true_false_0_false");
expect(cvo1.key).toBe("LC1#2@736_64_ZUE_6_5_96_124_false_2_false_true_false_0_false");

const cvo2 = new NodeViewObject(editorView, node, true);
expect(cvo2.key).toBe("#2@736_64_ZUE_6_5_96_124_false_2_true_true_false_0_false");
expect(cvo2.key).toBe("LC1#2@736_64_ZUE_6_5_96_124_false_2_true_true_false_0_false");
});

it("NodeViewObject - 001", () => {
dataService.loadNetzgrafikDto(NetzgrafikUnitTesting.getUnitTestNetzgrafik());
const note = noteService.getNoteFromId(3);
const cvo1 = new NoteViewObject(editorView, note);
expect(cvo1.key).toBe(
"#3@1312_160_64_192_<p><em>Folgendes</em></p>spannend<p><strong>FETT</strong>_Frabcodierter Text_false_0_false_false_0_false",
"LC1#3@1312_160_64_192_<p><em>Folgendes</em></p>spannend<p><strong>FETT</strong>_Frabcodierter Text_false_0_false_false_0_false",
);
});

Expand All @@ -217,7 +218,7 @@ describe("Editor-DataView", () => {
const ts = trainrunSectionService.getTrainrunSectionFromId(3);
const cvo1 = new TrainrunSectionViewObject(editorView, ts);
expect(cvo1.key).toBe(
"#3@1234_false_false_0_39_49_1_21_39_0_0_141_39_0_180_0_S_20_7/24_4_1_0_S_20_7/24_20_0_round_trip_false_true_false_false_false_false_false_false_false_true_true_true_false_false_true_true_true_0_false_2_true_true_true_1_false_true_true_0_false_true_true(130,80)(194,80)(254,80)(318,80)",
"LC1#3@1234_false_false_0_39_49_1_21_39_0_0_141_39_0_180_0_S_20_7/24_4_1_0_S_20_7/24_20_0_round_trip_false_true_false_false_false_false_false_false_false_true_true_true_false_false_true_true_true_0_false_2_true_true_true_1_false_true_true_0_false_true_true(130,80)(194,80)(254,80)(318,80)",
);
});

Expand All @@ -226,7 +227,7 @@ describe("Editor-DataView", () => {
const trans = nodeService.getNodeFromId(1).getTransitions()[0];
const cvo1 = new TransitionViewObject(editorView, trans, false);
expect(cvo1.key).toBe(
"#0@false_false_IC_60_7/24_1_3_0_IC_60_7/24_60_false_false_false(320,48)(324,48)(412,48)(416,48)",
"LC1#0@false_false_IC_60_7/24_1_3_0_IC_60_7/24_60_false_false_false(320,48)(324,48)(412,48)(416,48)",
);
});

Expand Down
5 changes: 5 additions & 0 deletions src/app/view/editor-main-view/data-views/editor.view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export class EditorView implements SVGMouseControllerObserver {
notesView: NotesView;
isMultiSelectOn = false;

getNetzgrafikLoadCounter = null;
addNode = null;
getNodePathToEnd = null;
addTrainrunSectionWithSourceTarget = null;
Expand Down Expand Up @@ -166,6 +167,10 @@ export class EditorView implements SVGMouseControllerObserver {
this.svgMouseController.destroy();
}

bindGetNetzgrafikLoadCounter(callback) {
this.getNetzgrafikLoadCounter = callback;
}

bindAddNode(callback) {
this.addNode = callback;
}
Expand Down
2 changes: 2 additions & 0 deletions src/app/view/editor-main-view/data-views/nodeViewObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ export class NodeViewObject {

static generateKey(editorView: EditorView, n: Node, isNodeStopNode: boolean): string {
return (
"LC" +
editorView.getNetzgrafikLoadCounter() +
"#" +
n.getId() +
"@" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ describe("Nodes-View", () => {
levelOfDetailService,
undefined,
positionTransformationService,
dataService,
);

new EditorView(
Expand Down
12 changes: 6 additions & 6 deletions src/app/view/editor-main-view/data-views/nodes.view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ export class NodesView {
if (dragTransitionInfo !== null) {
this.reconnectTransition(dragTransitionInfo, endNode);
} else {
const startNode: Node = this.editorView.trainrunSectionPreviewLineView.getStartNode();
const startNode = this.editorView.trainrunSectionPreviewLineView.getStartNode();
this.createNewTrainrunSection(startNode, endNode);
}
}
Expand Down Expand Up @@ -729,19 +729,19 @@ export class NodesView {
this.editorView.trainrunSectionPreviewLineView.getExistingTrainrunSection();
}
this.editorView.trainrunSectionPreviewLineView.stopPreviewLine();
if (startNode === endNode) {
if (!startNode || !endNode || startNode?.getId() === endNode?.getId()) {
Copy link
Copy Markdown
Contributor Author

@aiAdrian aiAdrian Feb 26, 2026

Choose a reason for hiding this comment

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

This change ensures that even in the worst-case scenario—if the rendering object based on the load counter malfunctions—this check would still be handled correctly. The old check should still be compared to the new, correct one. However, the issue of outdated objects in the rendering (SVG) is resolved by the counter.

Therefore, I recommend not reverting this change.

return;
}
if (existingTrainrunSection !== null) {
if (
existingTrainrunSection.getSourceNode() === startNode &&
existingTrainrunSection.getTargetNode() === endNode
existingTrainrunSection.getSourceNodeId() === startNode.getId() &&
existingTrainrunSection.getTargetNodeId() === endNode.getId()
) {
return;
}
if (
existingTrainrunSection.getSourceNode() === endNode &&
existingTrainrunSection.getTargetNode() === startNode
existingTrainrunSection.getSourceNodeId() === endNode.getId() &&
existingTrainrunSection.getTargetNodeId() === startNode.getId()
) {
return;
}
Expand Down
2 changes: 2 additions & 0 deletions src/app/view/editor-main-view/data-views/noteViewObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export class NoteViewObject {

static generateKey(editorView: EditorView, n: Note): string {
return (
"LC" +
editorView.getNetzgrafikLoadCounter() +
"#" +
n.getId() +
"@" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ describe("Notes-View", () => {
levelOfDetailService,
undefined,
positionTransformationService,
dataService,
);

new EditorView(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ export class TrainrunSectionViewObject {
cumulativeTravelTimeData[cumulativeTravelTimeData.length - 1].sumTravelTime;

let key =
"LC" +
editorView.getNetzgrafikLoadCounter() +
"#" +
d.getId() +
"@" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ describe("TrainrunSection-View", () => {
levelOfDetailService,
undefined,
positionTransformationService,
dataService,
);

new EditorView(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1969,7 +1969,7 @@ export class TrainrunSectionsView {
StaticDomTags.CONNECTION_TAG_ONGOING_DRAGGING,
false,
);
this.createNewTrainrunSectionAfterPinDropped(
this.handleTrainrunSectionAfterPinDropped(
TrainrunSectionsView.getNode(trainrunSection, atSource),
trainrunSection,
);
Expand Down Expand Up @@ -2562,19 +2562,19 @@ export class TrainrunSectionsView {
);
}

private createNewTrainrunSectionAfterPinDropped(endNode: any, trainrunSection: TrainrunSection) {
private handleTrainrunSectionAfterPinDropped(endNode: Node, trainrunSection: TrainrunSection) {
if (this.editorView.trainrunSectionPreviewLineView.getMode() === PreviewLineMode.NotDragging) {
return;
}

if (endNode === null) {
if (!endNode) {
this.editorView.deleteTrainrunSection(trainrunSection);
this.editorView.trainrunSectionPreviewLineView.stopPreviewLine();
return;
}

const startNode: any = this.editorView.trainrunSectionPreviewLineView.getStartNode();
if (startNode === endNode) {
const startNode = this.editorView.trainrunSectionPreviewLineView.getStartNode();
if (!startNode || startNode.getId() === endNode.getId()) {
this.editorView.trainrunSectionPreviewLineView.stopPreviewLine();
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ export class TransitionViewObject {

static generateKey(editorView: EditorView, transition: Transition, isMuted: boolean): string {
let key =
"LC" +
editorView.getNetzgrafikLoadCounter() +
"#" +
transition.getId() +
"@" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ describe("Transitions-View", () => {
levelOfDetailService,
undefined,
positionTransformationService,
dataService,
);

new EditorView(
Expand Down
14 changes: 14 additions & 0 deletions src/app/view/editor-main-view/editor-main-view.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {LevelOfDetailService} from "../../services/ui/level.of.detail.service";
import {ViewportCullService} from "../../services/ui/viewport.cull.service";
import {VersionControlService} from "../../services/data/version-control.service";
import {PositionTransformationService} from "../../services/util/position.transformation.service";
import {DataService} from "../../services/data/data.service";

@Component({
selector: "sbb-editor-main-view",
Expand Down Expand Up @@ -78,6 +79,7 @@ export class EditorMainViewComponent implements AfterViewInit, OnDestroy {
private levelOfDetailService: LevelOfDetailService,
private versionControlService: VersionControlService,
private positionTransformationService: PositionTransformationService,
private dataService: DataService,
) {
this.editorView = new EditorView(
this,
Expand Down Expand Up @@ -110,6 +112,16 @@ export class EditorMainViewComponent implements AfterViewInit, OnDestroy {
this.uiInteractionService.moveNetzgrafikEditorViewFocalPointObservable
.pipe(takeUntil(this.destroyed))
.subscribe((center: Vec2D) => this.moveNetzgrafikEditorViewFocalPoint(center));
this.dataService
.getNetzgrafikLoadedInfo()
.pipe(takeUntil(this.destroyed))
.subscribe((loadedInfo) => {
// Reset preview line view when new DTO is loaded to clear stale object references
// This fixes issue #851: prevents creating invalid trainruns when clicking nodes after import
if (!loadedInfo.load && !loadedInfo.preview) {
this.editorView.trainrunSectionPreviewLineView.stopPreviewLine();
}
});
}

@HostListener("window:resize", ["$event"])
Expand Down Expand Up @@ -163,6 +175,8 @@ export class EditorMainViewComponent implements AfterViewInit, OnDestroy {
}

bindViewToServices() {
this.editorView.bindGetNetzgrafikLoadCounter(() => this.dataService.getNetzgrafikLoadCounter());

this.editorView.bindAddNode((positionX: number, positionY: number) =>
this.nodeService.addNodeWithPosition(positionX, positionY),
);
Expand Down