gnembon / fabric-carpet

Fabric Carpet
MIT License
1.73k stars 275 forks source link

[Scarpet Bug?] Global lists in some events behaving randomly #459

Closed altrisi closed 4 years ago

altrisi commented 4 years ago

I first was going to call this "Events don't update global variables after being called again" + "Events can't save global variables", but I can't get it to do only that right now. This was checked only with __on_player_connects() and __on_player_disconnects() events, and with Carpet players. Also only with arrays (if I remember correctly).

I may just be doing something wrong, sorry if that is the case, but I've tried in many different ways and I'm almost sure something is going wrong.

The issue

(those) Events will not save global variables (outside of them). That can be checked by running /script in [name] globals or /script in [name] run [global_name], where the global (s) will show as if only initialized (in this case []). However, it can be accessed from other events, which may use any version of the variable.

For example, in the attached script I repopulate a global variable every time a Carpet player joins with all Carpet players. (I have some "holder" variables since I spent a lot of time trying to find what was going on, unsuccessfully). Then, when a Carpet player is killed (disconnects), instead of repopulating the variable it shrinks the variable (in this case, a different variable, to illustrate that it can pickup any state of the original) and it should print the new variable to the user. However, when doing so, it may shrink the state of the variable as if recently initialized, or at any point of the app being present.

To reproduce with the attached script, load it and spawn one or more Carpet players (use /script run run('player Bot'+round(rand(40))+' spawn at [safe coords]') if not feeling imaginative about names or want to create a cmd block). Check script's globals (/script in [name] globals), you'll see the variable is empty although it had been printed previously in chat. With more than one Carpet player, kill one of them (can be done by /kill @r[name=!(yourign)] too). You'll see the 1 bot deleted plus some data, that even though /script in [name] globals said it was empty, may be populated. Try killing another one. Since disconnects() doesn't rewrite the original variable, in theory you should see similar data, just with a different removed one, but in practice it can be any state of the variable. You can try to keep adding extra players and removing manual ones, the variable may or may not be updated. You can also try killing all Carpet players and observe again random data being printed.

Mostly for the sake of doing it, I've recorded a video with the procedure mentioned above and the bug occurring. You can find it here.

The script used was the following:

global_botList = [];

printr(msg)->(print(player('altrisi'),msg)); //Change this

__on_player_connects(p) -> (
    if(p~'player_type' == 'fake',
        bots=filter(player('*'),_~'player_type'=='fake');
        botListHolder = [];
        for(bots,
            botData = [_~'name'];
            put(botData,1,map(pos(_),round(_*100)/100),'extend');
            put(botListHolder,_i,botData);
        );
        //Trying to see if it works when schedule() d. It doesn't seem to. Can be changed to inline with same behaviour
        schedule(0,_(outer(botListHolder))->(
            global_botList = botListHolder;
            printr(global_botList);));

    );
);

__on_player_disconnects(player,reason) -> (
    logger('[SC] '+player~'name'+' left for '+reason+' (it is a '+player~'player_type'+')');
    if(player~'player_type' == 'fake',
        //This is on purpose not editing the same global, for demonstration purposes
        global_connectedBots = filter(global_botList,_:0 != player~'name'); 
        printr('1 bot deleted. New data: '+global_connectedBots);
    );
);

From app commands, globals work correctly (or at least don't behave weirdly, although I'm not sure if I've tested arrays). Tested in Carpet 1.4.8, 1.4.9 and a clean (loader+carpet only) 1.4.10 server.

PS: After hours thinking if reporting it or not by still thinking I may be doing something wrong, I've decided to send it. Sorry again if it's just me.

gnembon commented 4 years ago

Yes, and that's a really good example of showing what a difference between a player scope app is and global scope app is. You have a player scoped app, meaning every player will have their own set of functions and globals not interfering with each other. If you define your app as global, this will keep only one set of globals and fundefs per app, doing what you probably want to do. by typing /script in <app> globals you show your globals. To snoop other player's globals, you can run /execute as <p> run script in <app> globals. There is a trick to see the 'server' portion for the app by /execute @e[type=!player,limit=1] run script in <app> globals since running as any other non-player entity runs your command as a server.

This is worth a wiki article to be honest because the example is prime