Found a possible bug.

Options
Hey guys,

To save me having to rewrite a whole bunch of information, I originally posted this problem on the Unity forum thinking that it might have been a problem with Unity. You can find the post here: http://answers.unity3d.com/questions/511041/strange-nullreferenceerror-when-dealing-with-hasht.html#answer-511121

To clarify, I am building a multiplayer game and utilising the customProperties of server objects such as the PhotonPlayer and Room. Because of this, I was waiting for a call to OnPhotonCustomRoomPropertiesChanged() to be notified when someone has joined the room and to change the properties of it.

From doing this, i noticed two problems, which I assume are maybe bugs?

1) Even though the properties were only updated once, by one client, the method OnPhotonCustomRoomPropertiesChanged() was called twice.
2) On the first call of OnPhotonCustomRoomPropertiesChanged() the customProperties of the room were not yet available, it was returning an empty hashtable. On the second call, the information was available. This has caused me to have to create a rerunning function to keep trying for the information until it is available.

If you would like to see some of the code, I would be happy to share.

Comments

  • Tobias
    Options
    I posted an answer at the unity page. Hopefully this helps you a bit.
    I will check the two issues you reported.
    Which PUN version are you using?
  • Tobias
    Options
    To test, I created a small scene and gui. Most likely I'll turn this into some tutorial, too.
    For the time being, maybe the attached project helps you a bit with experimenting.
  • Hey Tobias,

    My Photon version is currently 1.22 (this is a pretty old project now, only recently reopened it). I did check the changelog for your most recent versions and didn't see any changes to this problem before making the post though. So i've so far been assuming it's not yet been fixed.

    I posted a reply to your post on the Unity forums. The main thing to note is that I can confirm that it most certainly does make the call to OnPhotonCustomRoomPropertiesChanged more than once, even though the property changes were only set once. I can imagine that you might be right that the first call is being made on join/create of the room, as that would make sense (considering the possiblity of passing it as a parameter to CreateRoom()).

    Repeating what I said in the Unity forum, if you do manage to update this in a patch, please do stick a post in here to let us know. It would make updating worth the hassle! =p
  • Tobias
    Options
    Thanks for posting there AND here.

    I'm not sure if I should change the behaviour in an update. It's potentially a breaking one.
    It's true that calling OnPhotonCustomRoomPropertiesChanged when nothing was set, is not very useful. But when I skip this call, there won't be one at the beginning and someone might miss that.

    I don't think it's very hack-ish if you check props for being null first. It depends on how you use them. You could set more and more over time while playing. As that is perfectly legal, in some workflows the client even must check for null.

    I got to think about the options and might change this later.

    If you really use PUN 1.22, you're up to date.
  • Aye, don't get me wrong. Null checking has been drilled into me as much as the alphabet has. It's more the fact that the way I'm having to do it is seriously upsetting the work-flow of the programming.

    Basically, i'll start off looking for a room. Each room will have a set of parameters which will help me find the best room for a certain player. Instead of simply doing something like:

    //while in lobby
    foreach(RoomInfo ri in PhotonNetwork.GetRoomList())
    {
    myCustomPropertiesClass mcpc = new myCustomPropertiesClass();
    mcpc.setProperties(ri.customProperties);

    //now we have the properties, work out whether this one is good, if so:
    PhotonNetwork.JoinRoom(ri.name);
    }

    I would need to do this:

    void findARoom()
    {
    //while in lobby
    foreach(RoomInfo ri in PhotonNetwork.GetRoomList())
    {
    myCustomPropertiesClass mcpc = new myCustomPropertiesClass();
    //the setProperties method has now been changed to return false if any of the values in the hashTable are null
    if(!mcpc.setProperties(ri.customProperties))
    {
    //now i need to rerun the function, which means that I have to upset the workflow of the rest of the programming
    Invoke("findARoom",0.5f);
    return;
    }

    //now we have the properties, work out whether this one is good, if so:
    PhotonNetwork.JoinRoom(ri.name);
    }
    }

    Now, this might not look like a big problem doing this just once, but because of the nature of the type of game i'm building, I need to make many changes to the customProperties of players and rooms, and the other players will be waiting on the OnPhotonCustomRoomPropertiesChanged and every single time I want to get the changed properties of just check the properties a I need to build a rerunning method and stop everything else from happening until I get a value. Like I said in the other post, i'm doing things completely different now, however the programming ended up incredibly confusing and messy simply due to the number of loop holes you had to jump through and rerun until you updated and got the required custom propertied.

    I totally understand where you're coming from though, this may well be an issue that we need to live with as developers using Photon simply due to the fact that others have probably actually went ahead and built their applications using the above method, however hopefully this gives you a clearer picture of the problems that is causes.

    If you do make the changes it would be great to know as I certainly plan to continue using Photon in future applications.
  • Tobias
    Options
    Err.
    No.
    You don't have to re-run that method.

    There are plenty of ways to fix this without going the recursion way.
    Actually, you already established a "while in lobby" block. Maybe in Update().


    [code2=csharp]//in lobby, on room list update do

    myCustomPropertiesClass mcpc = new myCustomPropertiesClass();

    foreach(RoomInfo ri in PhotonNetwork.GetRoomList())
    {
    if (!CustomPropsSet(ri)) continue; // voila. all rooms without props are excluded already

    mcpc.setProperties(ri.customProperties);
    //now we have the properties, work out whether this one is good, if so
    PhotonNetwork.JoinRoom(ri.name);
    }[/code2]

    [code2=csharp]// to check custom props and if the room is fitting
    public bool CustomPropsSet(RoomInfo ri)
    {
    // check all needed values here
    }[/code2]

    While writing this, I noticed the first time you are doing this in the lobby. The callback for lobby updates is not OnPhotonCustomRoomPropertiesChanged. That is only called in a room.
    There is a callback for updates of the room list in the lobby: OnReceivedRoomListUpdate. This in turn doesn't get called while in a room.

    There is yet another way to solve this even more elegantly (I think): You can use Photon's matchmaking.
    There is a JoinRandomRoom method which takes a hashtable as filter parameter. If you use a filter, all key-value pairs must fit or no room is being joined and OnPhotonRandomJoinFailed will be called. In there, you can relax the filter by picking more common key-values or removing less important ones. Repeat until no filter is used anymore or until you want to simply create a room and let others join.
  • I don't tend to program these types of things within an Update loop, simply because linearly programming something (running a program from top to bottom with no loops) which technically should only happens once makes much more sense (in my mind, call me old fashioned...).

    With that being said, during my play-around with the custom properties, when I was creating a room using the CreateRoom method, I was utilising the (i think?) 4th and 5th parameters of the method. The 4th being the custom properties to assign, and the 5th being the properties to list in the lobby. This worked completely fine. I was able to find a specific type of room for a player using this method without a problem, it was when it came to then updating the room properties (when a player joins the room) and then waiting for the callback before doing something else.


    Yeah, the matchmaking was my original intention. However the problems with the OnPhotonCustomRoomPropertiesChanged callback happening before properties were available (this is while you're in the room) and the player properties were ultimately the reasons i've had to go for a more customised method.

    Thanks again for the help though. Always appreciated.