asadm / playroom-unity

9 stars 1 forks source link

InitOptions bug #44

Closed asadm closed 5 months ago

asadm commented 5 months ago

In PlayroomKit.InitOptions, i'm setting maxPlayersPerRoom to 2. But, up to four is still able to join lobby.

SaadBazaz commented 5 months ago

@asadm If possible, please link the Chat.


We gotta reproduce this.

This could be because of the nullable property (even though we need it) here: https://github.com/asadm/playroom-unity/blob/main/Assets/PlayroomKit/PlayroomKit.cs#L42

This is how the options are being converted: https://github.com/asadm/playroom-unity/blob/main/Assets/Plugins/PlayroomPlugin.jslib#L35

Maybe we're losing information somewhere in the middle? @momintlh

Can we test this out (print logs?) and see?

momintlh commented 5 months ago

@asadm If possible, please link the Chat.

We gotta reproduce this.

This could be because of the nullable property (even though we need it) here: https://github.com/asadm/playroom-unity/blob/main/Assets/PlayroomKit/PlayroomKit.cs#L42

This is how the options are being converted: https://github.com/asadm/playroom-unity/blob/main/Assets/Plugins/PlayroomPlugin.jslib#L35

Maybe we're losing information somewhere in the middle? @momintlh

Can we test this out (print logs?) and see?

I'll do some tests.

asadm commented 5 months ago

https://discord.com/channels/997752993598419044/1128151935669764096/1203515785331347457

momintlh commented 5 months ago

I just did a build test with setting maxPlayersPerRoom = 2, and this is the JSON coming from C# to JS: image

My InsertCoin:

PlayroomKit.InsertCoin(new PlayroomKit.InitOptions()
        {
            maxPlayersPerRoom = 2,
            defaultPlayerStates = new() {
                        {"score", -500},
                    }
        }, () =>
        {
            PlayroomKit.OnPlayerJoin(AddPlayer);
            PlayroomKit.SetState("score", score);
        });

and Only 2 players can join:

P1

image

P2

image

and P3: image

@asadm @SaadBazaz

SaadBazaz commented 5 months ago
Screenshot 2024-02-04 at 19 26 11

@asadm - I noticed that our latest release is 3 weeks ago, despite merges and updates in the middle. Do you think this is the reason?

We may need to add a tag to our next PR to trigger it. Should we add one in #43?


@momintlh Can you ask the developer in Discord for a repro?

asadm commented 5 months ago

is our current main stable? i can do a release once we are stable.

SaadBazaz commented 5 months ago

@momintlh Test the current main. Don't add RPC to it yet; that can cause instability for now.

momintlh commented 5 months ago

I tested the latest release and had no issues with setting maxPlayersPerRoom = 2. 3rd Player: image

momintlh commented 5 months ago

After a detailed discussion with the OP, the issue ended up being that options parameter was not passed correctly to InsertCoin().

In v0.0.10 the signature is: InsertCoin(callback, options)