Skip to content

Conversation

@damienmarchal
Copy link
Contributor

@damienmarchal damienmarchal commented Jun 13, 2025

@EulalieCoevoet and @bakpaul, feel free to add other tests (including one representing broken usage).

As it is the entry point of Sofa... it must be very robust.

@fredroy fredroy changed the title Add the unified "add" in Sofa.Core.Node [STLIB] Add the unified "add" in Sofa.Core.Node Jun 23, 2025
if params["name"] in self.children or params["name"] in self.objects:
names = {node.name.value for node in self.children}
names = names.union({object.name.value for object in self.objects})
params["name"] = findName(params["name"], names)
Copy link
Contributor

Choose a reason for hiding this comment

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

Throw warning if name already exists

Copy link
Contributor

@hugtalbot hugtalbot Jul 17, 2025

Choose a reason for hiding this comment

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

Suggested change
params["name"] = findName(params["name"], names)
# send a warning here : Sofa.Helper.msg_warning(target, message, frameinfo.filename, frameinfo.lineno)
params["name"] = findName(params["name"], names)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok in a future PR :)

Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>
@damienmarchal damienmarchal merged commit a01f75d into 25_04_work_on_new_prefabs Jul 17, 2025
6 of 8 checks passed
bakpaul pushed a commit that referenced this pull request Oct 28, 2025
* Add Sofa.Core.Node.add with preliminary test

* rename the "add" function and make it private

* Apply suggestions from code review

Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>

---------

Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants