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

Fix: expose AttributeOptions #10407

Merged
merged 2 commits into from Apr 9, 2024
Merged

Fix: expose AttributeOptions #10407

merged 2 commits into from Apr 9, 2024

Conversation

ivanpopelyshev
Copy link
Collaborator

The problem in existing project:

For v7 I always overrode geometry attributes in constructor:

class MyGeom extends Geometry {
    constructor() {
         this.buffer = new Buffer();
         this.addAttribute();
    }
}

and that evolved to function that just takes care of buffer:

class MyGeom extends Geometry {
    constructor() {
         this.buffer = new Buffer();
         this.initGeom();
    }
    initGeom() {
         this.addAttribute();
    }
}

initGeom() can also used for similar geometries that have same attribute list, but completely different set of fields, i just call MyGeom.prototype.initGeom.call(this)

know this approach is not possible because attributes has to be passed to super(). Thus, I have to

class MyGeom extends Geometry {
    constructor() {
         const buffer = new Buffer();
         super({ BIG_F_ATTRIBUTE_LIST })
         this.buffer=buffer;
    }
}

that's certainly not ok , so I added function, again:

class MyGeom extends Geometry {
    constructor() {
         const buffer = new Buffer();
         super({ attributes: initGeom(buf) });
    }
    static initGeom(buf: Buffer): AttributeOptions {
         return { a_attr1: {} , a_attr2: {}, }
    }
}

thus, initGeom can be used for other geometry classes.

However, that is not passing through typescript, I need AttributeOptions exposed to set the type of initGeom manually, it cannot be infered!

Copy link

codesandbox-ci bot commented Apr 1, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ac9103a:

Sandbox Source
pixi.js-sandbox Configuration

@Zyie Zyie added the v8 label Apr 2, 2024
@Zyie Zyie requested a review from GoodBoyDigital April 3, 2024 13:15
@Zyie Zyie added the ✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t label Apr 4, 2024
@Zyie Zyie changed the title Expose AttributeOptions Chore: expose AttributeOptions Apr 4, 2024
@Zyie Zyie changed the title Chore: expose AttributeOptions Fix: expose AttributeOptions Apr 4, 2024
@Zyie Zyie added this pull request to the merge queue Apr 9, 2024
Merged via the queue into dev with commit 87fbc00 Apr 9, 2024
4 checks passed
@Zyie Zyie deleted the expose-attribute-type branch April 9, 2024 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t v8
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants