citizenfx / fivem

The source code for the Cfx.re modification frameworks, such as FiveM, RedM and LibertyM, as well as FXServer.
https://cfx.re/
3.47k stars 2.05k forks source link

BaseClass with EnableOnLoad(false) still loads ticks, events, etc. by registering them without the attributes #2476

Open AvarianKnight opened 4 months ago

AvarianKnight commented 4 months ago

What happened?

If a class inherits from BaseClass and has the attribute [EnableOnLoad(false)] calling register functions Tick += or EventHandlers[eventName] += will still schedule the tick & will still bind the event handler.

I would assume this works the same for exports but I couldn't figure out how to register them via Exports.Add in mono_rt2

Expected result

The class class inheriting BaseClass should only add Event/Tick/Nui whenever script.Enable() is called

Reproduction steps

Client.cs

using CitizenFX.Core;
using CitizenFX.RedM.Native;

public class Main : BaseScript
{
    Client m_client;

    public Main()
    {
        m_client = new Client();
    }

    void CallUi()
    {
        Natives.SendNuiMessage("{}");
    }

    [Tick]
    async Coroutine ToggleScript()
    {
        await Wait(5000);
        Debug.WriteLine("Enable");
        m_client.Enable();
        CallUi();
        await Wait(5000);
        Debug.WriteLine("Disable");
        m_client.Disable();
        CallUi();
    }
}

[EnableOnLoad(false)]
public class Client : BaseScript
{
    bool m_printed = false;

    async Coroutine TickFunction()
    {
        if (!m_printed)
        {
            Events.TriggerEvent("TestEvent");
            m_printed = true;
            Debug.WriteLine("Tick gets called!");
        }
    }

    async Coroutine EventFunction()
    {
        Debug.WriteLine("Event gets called");
    }

    override protected void OnDisable()
    {
        m_printed = false;
    }

    override protected void OnEnable()
    {
        m_printed = false;
    }

    public Client()
    {
        Tick += TickFunction;
        EventHandlers["TestEvent"] += EventFunction;

        RegisterNuiCallback("NuiBaseCB", new Action<IDictionary<string, object>, Callback>((IDictionary<string, object> nuiData, Callback cb) =>
        {
            cb("base cb 1");
        }));

        BaseScript.AddNuiCallback("NuiStaticBaseCB", new Action<IDictionary<string, object>, Callback>((IDictionary<string, object> nuiData, Callback cb) =>
        {
            cb("static cb 2");
        }));
    }

    [NuiCallback("NuiCB")]
    void EventCallback(IDictionary<string, object> nuiData, Callback cb)
    {
        cb($"attribute cb 3");
    }
}

index.html

<!doctype html>
<html>
    <script>
        const callbackTests = ["NuiCB", "NuiBaseCB", "NuiStaticBaseCB" ];

        window.addEventListener("message", () => {
            for (const test of callbackTests) {
                console.log(`Triggering ${test}`);
                fetch(`https://${GetParentResourceName()}/${test}`, {
                    method: 'POST',
                    headers: {
                        'Content-Type': 'application/json; charset=UTF-8',
                    },
                    body: JSON.stringify({
                        ItemId: 'my-item-2',
                        Test: {
                            TestValue: "test",
                        }
                    })
                }).then(resp => resp.json()).then(resp => console.log(resp));
            }
        })

    </script>
</html>

Importancy

There's a workaround

Area(s)

ScRT: C#

Specific version(s)

N/A

Additional information

A simple fix for this would probably be to remove the Schedule() call for Ticks and remove the EventsManager.AddEventhandler call, and returning after m_nuiCallbacks if the script is not enabled.

Initialize() should probably only set the state to State.Initialized and call Enable() again after it finishes initializing attribute values and let Enable() handle scheduling the Tick/Nui/Event/Export/Commands.

thorium-cfx commented 4 months ago

I read the conversation that led to this issue and to that the answer is: yep [EnableOnLoad(false)] implies not activating all instances' ticks, events, etc. at construction. So this is indeed a bug due to an oversight.

This'll need to be fixed/tweaked