engine_fragment icon indicating copy to clipboard operation
engine_fragment copied to clipboard

Overwritten clone() method on FragmentsGroup throwing error

Open alvpickmans opened this issue 1 year ago • 0 comments

Describe the bug 📝

FragementsGroup overrides the THREE.Group.clone() method by throwing an error, informing to use instead FragmentsGroup.cloneGroup().

https://github.com/ThatOpen/engine_fragment/blob/b8121f8882c77bcc4adb0a7557cab5e7c1bd7ef3/packages/fragments/src/fragments-group.ts#L450-L452

On applications dealing with different file sources, it might be desirable to provide a default, expected implementation for clone() by cloning the whole group, rather than forcing a conditional branch to test if the Object3D instance is in fact a FragmentsGroup.

  clone(_recursive?: boolean): any {
    return this.cloneGroup(null);
  }

A quick workaround is to overwrite the clone function using prototype

FragmentsGroup.prototype.clone = function clone() {
  return this.cloneGroup();
};

Happy to open a PR if it is an acceptable solution.

Reproduction ▶️

No response

Steps to reproduce 🔢

No response

System Info 💻

Binaries:
    Node: 21.7.3 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.22 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 10.5.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Chromium (129.0.2792.89)
    Internet Explorer: 11.0.22621.3527
  npmPackages:
    @thatopen/components: ^2.3.15 => 2.3.15
    three: ^0.169.0 => 0.169.0

Used Package Manager 📦

yarn

Error Trace/Logs 📃

No response

Validations ✅

  • [X] Read the docs.
  • [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • [X] Make sure this is a repository issue and not a framework-specific issue. For example, if it's a THREE.js related bug, it should likely be reported to mrdoob/threejs instead.
  • [X] Check that this is a concrete bug. For Q&A join our Community.
  • [X] The provided reproduction is a minimal reproducible example of the bug.

alvpickmans avatar Dec 05 '24 09:12 alvpickmans