rbxts-flamework / core

Flamework is an extensible game framework. It requires typescript and offers many useful features.
MIT License
101 stars 8 forks source link

Default attributes not set on component #47

Closed hatmatty closed 2 years ago

hatmatty commented 2 years ago
import { OnStart } from "@flamework/core";
import { Component, BaseComponent } from "@flamework/components";

@Component({
    defaults: {
        attr: 1,
    },
})
export class component
    extends BaseComponent<{
        attr: number;
    }>
    implements OnStart
{
    onStart() {
        print(this.attributes.attr); // 1
        print(this.instance.GetAttribute("attr")); // nil
    }
}
Zyrakia commented 2 years ago

I believe this is intentional, modifying the attributes object will not result in any changes on the actual instance. Invalid or missing attributes on the instance are only ignored, not modified.

Fireboltofdeath commented 2 years ago

As said by @Zyrakia, this is intentional behavior. I'm open to hearing use cases on why you want the defaults to be added onto the instance, though.

hatmatty commented 2 years ago

For instance let’s say you have a server-sided component that has a default attribute and is connected to a tag. On the client you might want to get that component’s instance by tag and see it’s default attribute. You could just set the default attributes by hand but this is just a bit of quality of life and I don’t think it would be too costly.

Also I might be missing something but @Zyrakia said “modifying the attributes object will not result in any changes on the actual instance“ but on the documentation it states “The attributes object on components is mutable, so you can change the value and it'll change in both the component's copy and on the instance.”

So if the defaults are setting the attributes on the attributes object, and the attributes object is linked to the instance, it would appear from the documentation that defaults set attributes on the instance.

jackTabsCode commented 2 years ago

I agree, this behavior is both undefined and confusing and limits my use case. They should be synced IMO, otherwise the “attributes” name is misleading

Fireboltofdeath commented 2 years ago

https://github.com/rbxts-flamework/components/commit/2cee8e9dff00219890d25e117fae9804a485aa92