Skip to content

WebotsJS: Comparison without value#5979

Merged
BenjaminDeleze merged 4 commits intodevelopfrom
fix-regeneration-bug
Mar 13, 2023
Merged

WebotsJS: Comparison without value#5979
BenjaminDeleze merged 4 commits intodevelopfrom
fix-regeneration-bug

Conversation

@BenjaminDeleze
Copy link
Copy Markdown

Introduce a more permissive definition of equals to use in the context of restrictions.

Example:
In the AddLaneRoadSegment proto, the lines parameter is restricted and accept only one type of node: RoadLine { }.

It seems to work as intended at first glance: we can insert new RoadLine { }.
However, if we modify a RoadLine { }, let's say we change the line type from dashed to double and later we trigger a regeneration of the proto, it will not work.
This is the case because now we are comparing RoadLine { } with RoadLine { type double }

@BenjaminDeleze BenjaminDeleze added the bug Something isn't working label Mar 13, 2023
@BenjaminDeleze BenjaminDeleze added this to the R2023b milestone Mar 13, 2023
@BenjaminDeleze BenjaminDeleze self-assigned this Mar 13, 2023
@BenjaminDeleze BenjaminDeleze requested a review from a team as a code owner March 13, 2023 07:40
Copy link
Copy Markdown
Contributor

@ad-daniel ad-daniel left a comment

Choose a reason for hiding this comment

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

seems overkill to even use value.equals at this point, to be honest. In the context of restrictions it should suffice to check that the .url coincides between the two nodes. That should ensure that the PROTO being inserted is the same one as the allowed one (same url), and in the case of basenodes, the url is the nodes name itself, so it should work as well.

@BenjaminDeleze
Copy link
Copy Markdown
Author

seems overkill to even use value.equals at this point, to be honest. In the context of restrictions it should suffice to check that the .url coincides between the two nodes. That should ensure that the PROTO being inserted is the same one as the allowed one (same url), and in the case of basenodes, the url is the nodes name itself, so it should work as well.

Seems good to me, I will do the change

Copy link
Copy Markdown
Contributor

@ad-daniel ad-daniel left a comment

Choose a reason for hiding this comment

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

Thank you

@BenjaminDeleze BenjaminDeleze merged commit b4891a4 into develop Mar 13, 2023
@BenjaminDeleze BenjaminDeleze deleted the fix-regeneration-bug branch March 13, 2023 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Development

Successfully merging this pull request may close these issues.

2 participants