[1.7.10] Asking questions about WorldSavedData and Messages/Packets

[M_Sc_Student]

Hi everyone,

 

I recently learned about WorldSavedData for global data structures and I have some questions about it.

 

First of all is there any serious downside to using some kind of Singleton for my WorldSavedData? I know that Singletons have some serious downsides when not handled correctly.

 

I deleted the creation of the Singleton in the getInstance(), because we now have a load-method which creates it.

 

public static DataManager getInstance(){
return DataManager.instance;
}

public static void load(World world) {                                                                 
if (instance == null) {                                          
	instance = (DataManager)world.mapStorage.loadData(DataManager.class, IDENTIFIER);
	if (instance == null) {
		instance = new DataManager(IDENTIFIER);
		world.mapStorage.setData(IDENTIFIER, instance);
	}                                                                                              
}                                                                                                         
}                                                                                                      

 

Not yet sure if I should keep the doublecheck with instance == null. The load method is called in a custom CommonProxy EventHandler.

 

public void onWorldLoad(Load event)
{
	DataManager.load(event.world);
}

 

Not exactly sure if I only load that on the ServerProxy. Maybe.

 

And since the Manager is extending WorldSavedData, it has big read and write methods with a lot of Data. I learned already that the Data is automatically synced when the player joins the server (not sure yet, if its because the EventHandler is in CommonProxy). Is there any other important event when it syncs automatically, that I should know about?

 

Is it recommended to simply use the write and read methods to send the Data to the Client when he asks for it?

Btw. people said packets are used to send WorldSavedData between server and client. I assume I can simply use Messages(?)

 

simplified example:

GUI:
@Override                                                   
protected void actionPerformed(GuiButton button) {          
if(button.id == 2){ // Refresh                          
	Main.network.sendToServer(new RefreshMessage());  
}
}
-------------------------------------------------------------
RefreshMessage:
...stuff...
public class RefreshMessage implements IMessage {
...stuff...
public static class ServerHandler implements IMessageHandler<RefreshMessage, IMessage> {
	@Override
	public IMessage onMessage(RefreshMessage message, MessageContext ctx) {
		NBTTagCompound tagCompound = new NBTTagCompound();
		DataManager.getInstance().writeToNBT(tagCompound);
		return new DataMessage(tagCompound);
	}
}

}
-------------------------------------------------------------
DataMessage:
...stuff...
public class DataMessage implements IMessage {
private NBTTagCompound tagCompound;

public DataMessage() {	}
public DataMessage(NBTTagCompound c) {
	this.tagCompound = c;
}

@Override
public void fromBytes(ByteBuf buf) {
	this.tagCompound = ByteBufUtils.readTag(buf);
}

@Override
public void toBytes(ByteBuf buf) {
	ByteBufUtils.writeTag(buf, this.tagCompound);
}

public static class ClientHandler implements IMessageHandler<DataMessage, IMessage> {
	@Override
	public IMessage onMessage(DataMessage message, MessageContext ctx) {
		DataManager.getInstance().readFromNBT(message.tagCompound);
		DataManager.getInstance().markDirty();
		return null;
	}
}
}

 

That's the smallest Message-class I ever wrote.

 

With my old mod where I used Tile Entities, I simply changed them on the server and then used .getWorldObj().markBlockForUpdate() to send them to the clients, which I can't use anymore. Also it wouldn't be very smart to send the Data to all clients anyway, since it would be a lot of unnecessary traffic.

 

Thanks in advance for all answers. I will come back tomorrow or the day after.

[M_Sc_Student]

Do not cache the WorldSavedData in a static field, it will break a lot. Use a method like this and call it every time you need access to your WorldSavedData. There is no need to explicitly load your data in WorldEvent.Load.

 

The data is actually not synced at all. If you want it to sync to the client, you have to send packets. Your observation of the "automatic syncing" is most likely a side-effect of the static instance field I mentioned earlier. This causes the data to be shared between client & server thread in single player, which is a very bad idea. Moreover it will not work on a dedicated server at all.

 

Hi! I saw your source code in a tutorial when I googled WorldSavedData and my load-method is actually inspired by your get-method. You are probably right about the static field being a bad choice, but let me explain one thing real quick. This data structure can get huge, depending on how many players want to trade and create offers. Its basicly a List of objects with multiple Strings, coordinates, Itemstacks and potentially a bunch of other stuff. And my first thought was "loading it from the mapstorage over and over again seems like a big overhead, if I have it accessible in the RAM already without any additional loading.". I don't doubt that you are right with your advice, but can you lead me to some examples where I can see what this static field can break in minecraft (+mods)? This stuff is great to justify decisions in the documentation, y'know? I write my master thesis about this project (not in English btw.)

 

To the "automatic synching": I tested it again with a new world, and it's not synced - it was not a side effect on the static field, but that I used an old world and maybe I even created some data on the client. So I don't know what I did yesterday, but it's clearly not synced  I just saw some "readfromNBT" in my Client-log yesterday and "writeToNBT" in the server-log, so I thought it was sending it to the client on joining, but it's gone now. But even with the static field it works very well at the moment, even on dedicated. I click on the "refresh"-Button, it sends the refreshmessage, receives the DataMessage and refreshes the List.

 

Thank you very much for your answer.

[M_Sc_Student]

MapStorage keeps the thing in RAM already.

Okay thanks. I had to look into "loadData" to see why it says "load" instead of "get", because that made me think that it costs time to do it. It uses a FileInputStream, but only if the SavedData is null, so I guess only the very first time it takes a bit longer.

 

Fire up a dedicated server and try it out. It will break horribly. Or have a List in your WorldSavedData and try to add to it and in other parts of the code iterate through it. It will crash randomly at some point because you are accessing it from two threads.

Okay, so we are talking about Thread-safety. I will definitely change it. Just theoretically, would it help to make getInstance and/or some other methods synchronized ? And Why?

 

One last question, is it recommended to use the NBTTagCompound in the DataMessage like I stated in my third code block ?

 

Sorry if I ask too much and thanks again for your answers.

[M_Sc_Student]

Just theoretically, would it help to make getInstance and/or some other methods synchronized ? And Why?

No, it wouldn't. The issue is not the get method. The issue is that you are using one data structure from two threads. But the fact that it is two threads is just an implementation detail. When not playing single player but on a dedicated server these two threads run on completely distinct machines so the static field is suddenly not shared anymore. Relying on the fact that it is shared in any way is a bad idea.

I don't rely on that, though. Whenever I want to change the data structure from the Client, I would send a message to the Server and change it in the onMessage-method. In this case I only see the issue with different Threads. Normal DataStructures don't even have that problem with multiple Threads afaik, unless e.g. you don't take care of your Singletons. But maybe I'm missing something.

 

 

One last question, is it recommended to use the NBTTagCompound in the DataMessage like I stated in my third code block ?

You can, yes. But it would be more efficient if you were to only encode the data you actually need on the client directly into the ByteBuf.

 

I think that greatly depends on the use-case. If I send the whole Data-Structure, I can view every detail on the client and can let the player "Search"&"Filter" stuff, without any other communication. Then if he wants to answer to one entry I could use a different message that only checks the relevant things etc.

 

But I will think about it a bit more and maybe I can reduce the data for the RefreshMessage answer.

[M_Sc_Student]

The client/server discrepancy is only one issue if you have a static instance field. It will cause many more problems if you consider multiple dimensions. In general, please don't do it unless you really really know what you are doing with Multithreading.

Sure, how I said, I will change it, I just wanted to take the chance to learn something And of course it helps me with my thesis.