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.

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

findLinkDest() searching in parent nodes #3449

Open
hugtalbot opened this issue Nov 2, 2022 · 1 comment
Open

findLinkDest() searching in parent nodes #3449

hugtalbot opened this issue Nov 2, 2022 · 1 comment
Labels
issue: discussion Open topic of discussion STC#14 Tasks for STC#14 coding sprint

Comments

@hugtalbot
Copy link
Contributor

hugtalbot commented Nov 2, 2022

First noticed in #3339 canCreate() in TopologicalMapping calls findLinkDest() which checks the link at creation and if not found, the search continues in parent nodes.

findLinkDestClass(const sofa::core::objectmodel::BaseClass* destType, const std::string& path, const sofa::core::objectmodel::BaseLink* link) override;

is looking for the class in the given path and if not found is looking in the parent Node.

So for example in this code:

    BaseMeshTopology* stin;
    inPath = "@../";
    context->findLinkDest(stin, inPath , nullptr);
    
    BaseMeshTopology* stout;
    outPath = "@./";
    context->findLinkDest(stout, outPath, nullptr);

if no MeshTopology is present in ./ for the output mesh, it can return a "valid" pointer to a mesh in the parent Node. Therefor you end up with the same topology in Input and Output.

Do you think it is a behavior error of findLinkDestClass or a bad usage in this case ?

@epernod @damienmarchal @fredroy

@hugtalbot hugtalbot added issue: discussion Open topic of discussion STC#14 Tasks for STC#14 coding sprint labels Nov 2, 2022
@damienmarchal
Copy link
Contributor

damienmarchal commented Nov 17, 2022

In general I think that explicit link is the way to go.
Automatic deduction in the current node is acceptable but searching in the full hierarchy is going far too much.

I would recommend to deprecate that feature, and progressively remove it.

EDIT: I was thinking to make a small PR to fix that...and well...after having looked at the code it is, as often, going to be a noodle untangling task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: discussion Open topic of discussion STC#14 Tasks for STC#14 coding sprint
Projects
None yet
Development

No branches or pull requests

2 participants