Skip to content

fix: simplify deletion logic #113

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.

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

Merged
merged 1 commit into from
Aug 4, 2025
Merged

fix: simplify deletion logic #113

merged 1 commit into from
Aug 4, 2025

Conversation

thomassedlmayer
Copy link
Collaborator

@thomassedlmayer thomassedlmayer commented Jul 25, 2025

This PR removes the redundant getDeletedSceneEntites function and the corresponding calls.

The current logic seems a bit confusing to me. It compares the actually created lane/lane boundary scene entities with the previous...Ids lists which track the existence of IDs in the actual OSI messages (even if they are turned off for visualization). Also, the IDs in the state already have been updated to the current state and don't reflect the previous state anymore which makes it even more confusing. The logic actually leads to the deletion of physical/logical lane networks when the config parameters change but also a lot of deletions even if nothing has to be deleted (e.g. when config and lane/lane boundary ID presence hasn't changed):

image

I tried to fix and simplify it by checking the showLogicalLanes/showPhysicalLanes config parameters when getDeletedEntities is called and passing an empty list if the parameter is false, which also leads to deletion of the corresponding lane network if it was previously present.

@myemural Could you check out the changes?

Signed-off-by: Thomas Sedlmayer <[email protected]>
@jdsika jdsika merged commit 8803754 into main Aug 4, 2025
1 check passed
@jdsika jdsika deleted the fix/lane-deletion-logic branch August 4, 2025 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants