KubeJS (Old)

KubeJS (Old)

3M Downloads

Persistent data fails to save strings from event getters/functions

floral-qua-floral opened this issue ยท 4 comments

commented

This is a pretty strange one to explain. When you save a string as a persistent value, whether or not it will correctly set the string depends on how the string was originally obtained or created:

  • If the string is hard-coded - i.e. event.server.persistentData.test_persist = "Foo"; - it will save as expected.
  • If the string is from a function in an event - i.e. event.server.persistentData.test_persist = event.player.getName(); - it will save as null.
  • If the string is from a getter in an event - i.e. event.server.persistentData.test_persist = event.player.name; - it will save as null.
  • If the string is the tostring-ed version of a different data type, it will save as expected.

Additionally, if at any point the string has been concatenated, it will save as expected regardless of its origin, even if it's being concatenated with another string from an event function/getter. Concatenation seems to be the only way to make a string "forget" its origin, as far as I can tell - passing it around as a function, setting variables to it, etc all failed to allow an affected string to save correctly. Other data types such as numbers do not seem to be affected by this issue.

I've designed a server script to test this. It might be useful for easily and consistently reproducing the issue. Here is its code:

onEvent("player.chat", event => {
	let cancel_chat = true;
	switch(event.message) {
		case "Reset":
			event.server.persistentData.test_persist = null;
			printChat(event, "Persistent data has been reset");
		break; case "Get":
			printChat(event, "Value is " + event.server.persistentData.test_persist);
		break;
		
		case "Set to event function without concat":				//Fails
			setPersistentTo(event, event.player.getName());
		break; case "Set to event function with concat":			//Succeeds
			setPersistentTo(event, event.player.getName() + "");
		break; case "Set to event getter without concat":			//Fails
			setPersistentTo(event, event.player.name);
		break; case "Set to event getter with concat":				//Succeeds
			setPersistentTo(event, event.player.name + "");
		break; case "Set to hard-coded":						//Succeeds
			setPersistentTo(event, "VALUE");
		break;
		default: cancel_chat = false; break;
	}
	if(cancel_chat) event.cancel();
});
function setPersistentTo(event, setToValue) {
	event.server.persistentData.test_persist = setToValue;
	printChat(event, `Set the persistent data to ${setToValue}. Now it is ${event.server.persistentData.test_persist}.`);
}
function printChat(event, message, colour) {
	event.server.runCommand(`/tellraw @a {"text":"${message}","color":"${colour}"}`);
}

The script is used by sending chat messages. Every time the script attempts to set the persistent variable, it will print a message in chat reading "Set the persistent data to STRING1. Now it is STRING2.". STRING1 is the exact value that was passed into the function; STRING2 is the value of the persistent variable after it has been set to STRING1. The expected behaviour, of course, is that these should be the same regardless of what string you pass into the function.

I've been testing on Fabric 1.18 (kubejs-fabric-1801.4.1-build.271, rhino-fabric-1800.1.7-build.94, architectury-3.4.9). I haven't tested in any other versions of the mod yet.

commented

Update: This also seems to have some consequences even outside the context of persistent data. Specifically, the array.includes() function doesn't seem to work properly when given a string that is affected by this bug. Example code:

const RESPOND_TO = ["floralQuaFloral", "Dinnerbone"]
onEvent("player.chat", event => {
	if(RESPOND_TO.includes(event.player.name) printChat(event, "Hello!"); //Never fires
}
function printChat(event, message, colour) {
	event.server.runCommand(`/tellraw @a {"text":"${message}","color":"${colour}"}`);
}

However, array.indexOf() works consistently regardless of the origin of the string. I don't know what other things might be affected by this.

commented

This issue was probably related to some things returing String (from java) compared to most things created in JS returning NativeString (from Rhino). If you concat anything it gets turned into a NativeString, which is probably why that works.

There were some changes to do with type equality for those so this may be fixed now. I will do some testing later.

commented

I may of found out why Set to event function without concat is failing.
At some point getName() was modified tor return a component, which is what is failing.
I can reproduce it (albiet in KubeJS 6.0) by replacing event.player.getName() with Text.of('text').

This means its probably a bug in how Rhino/KubeJS tries to convert Components to NBT (which is what persistentData actually is)

commented

New reproduction script:

function setPersistentTo(event, setToValue, extraMessage) {
	event.server.persistentData.test_persist = setToValue
	event.server.tell(`Set the persistent data to ${setToValue}. Now it is ${event.server.persistentData.test_persist}. ${extraMessage}`)
}
function chatHandler(event) {
 setPersistentTo(event, Text.of('test component'), "Plain components fail") //fails
 setPersistentTo(event, Text.of('test component').getString(), "Getting the string from the component succeeds") //succeeds
 setPersistentTo(event, Text.of('test component') + ' concatation', "Concatting the component (therefore force toString()ing it) works") //succeeds

}
onEvent('player.chat', chatHandler)

Also, the name returning a component would also mess up your includes example as a Component != String (maybe could add some special equality there tho??)