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

Component onAttributeChanged callbacks report incorrect oldValue on the server #35

Closed mwalk-dev closed 2 years ago

mwalk-dev commented 2 years ago

When an onAttributeChanged callback is registered on the server, the oldValue parameter passed to the callback is always the current attribute value instead.

Quick test case that can be dropped into a fresh project as a shared component. The client-side output is what you would expect, but the server-side output is not.

import { BaseComponent, Component } from "@flamework/components";
import { OnStart } from "@flamework/core";
import { RunService } from "@rbxts/services";

@Component({
    tag: "Test",
    defaults: {
        TestValue: 0,
    },
})
export class TestComponent extends BaseComponent<{ TestValue: number }> implements OnStart {
    constructor() {
        super();
        this.onAttributeChanged("TestValue", (newValue, oldValue) => print(`Value changed from ${oldValue} to ${newValue}`));
    }

    onStart(): void {
        if (RunService.IsServer()) {
            task.spawn(() => {
                while (true) {
                    this.attributes.TestValue = math.random(0, 99);
                    task.wait(5);
                }
            });
        }
    }
}
Fireboltofdeath commented 2 years ago

This bug is caused due to this this.attributes.TestValue = since Flamework uses this.attributes to get the previous value, so a workaround is to use this.instance.SetAttribute or storing the previous attribute value before you change it manually.

I'll likely store previous attributes in a weak table in the Components implementation instead of using this.attributes

Fireboltofdeath commented 2 years ago

https://github.com/rbxts-flamework/components/commit/43064c31397bfbc60d7bc255158ece1301b76b77