OpenDream icon indicating copy to clipboard operation
OpenDream copied to clipboard

Implements `ImportText()`

Open Zonespace27 opened this issue 11 months ago • 2 comments

Implements the ImportText() proc. Closes #1639

(Unchanged) Functionality

  • Can take multiple savefile insertions by adding ; to the 2nd argument
  • Can take a key-value pair by adding = in-between a string and a number/string in the 2nd argument
  • Can insert valueless entries
  • If you insert a key-value pair with an empty key (= 6), . will become equal to the value.

Parity Breaks

  • Changes the strange BYOND pseudo-errors into simple worldLogs
  • Requires there to be a space on both sides of an =. I have made this change because the overall functionality of the = was incredibly inconsistent, as shown below:
    • "foo = bar" is valid and fine
    • "foo= 1" causes an error (due to how ImportText() works, it will still insert foo\= into the savefile)
    • "foo =1" is valid and fine
    • "foo =bar" causes an error like bullet point 2

TODO

  • [x] Write unit tests
  • [x] Differentiate between having no value and having a null value

Shoutout to goon for being the only codebase I could find that uses this proc

Zonespace27 avatar Mar 05 '24 04:03 Zonespace27

I would very much like to see some much more complex test cases here. It seems like you're only handling single line imports, which is only a small part of the functionality. It should be able to handle complex savefile structures, including objects.

This test should pass:

/obj/savetest
	var/obj/savetest/recurse = null

/proc/RunTest()
	var/obj/savetest/O = new() //create a test object
	O.name = "test"

	var/savefile/F = new() //create a temporary savefile

	F["dir"] = O
	F["dir2"] = "object(\".0\")"
	F["dir3"] = 1080
	F["dir4"] = "the afternoon of the 3rd"
	F["dir6/subdir6"] = 321
	F["dir7"] = null
	F["list"] << list("1",2,"three"=3,4, new /datum(), new /datum(), list(1,2,3, new /datum()))

	var/total_export = F.ExportText()

	var/savefile/F2 = new()
	F2.ImportText("/",total_export)
	//object test
	ASSERT(F2["dir"].name == "test")
	//string test
	ASSERT(F2["dir2"] == F["dir2"])
	ASSERT(F2["dir4"] == F["dir4"])
	//int test
	ASSERT(F2["dir3"] == F["dir3"])
	//null test
	ASSERT(F2["dir7"] == null)
	//subdir test
	ASSERT(F2["dir6"] == null)
	ASSERT(F2["dir6/subdir6"] == 321)
	//list test
	ASSERT(F2["list"][1] == "1")
	ASSERT(F2["list"][2] == 2)
	ASSERT(F2["list"]["three"] == 3)
	ASSERT(F2["list"][4] == 4)
	ASSERT(istype(F2["list"][5], /datum))
	ASSERT(istype(F2["list"][6], /datum))
	//nested list
	ASSERT(F2["list"][7][1] == 1)
	ASSERT(F2["list"][7][2] == 2)
	ASSERT(F2["list"][7][3] == 3)
	ASSERT(istype(F2["list"][7][4], /datum))

amylizzle avatar Mar 06 '24 10:03 amylizzle

I would very much like to see some much more complex test cases here. It seems like you're only handling single line imports, which is only a small part of the functionality. It should be able to handle complex savefile structures, including objects.

Done, thank you for writing that!

Zonespace27 avatar Mar 06 '24 23:03 Zonespace27

Fairly sure I came about this the wrong way. I haven't had a lot of motivation recently, so I'm closing this for the time being. Might come back to it someday, but I don't want to squat on this proc's implementation

Zonespace27 avatar Jun 10 '24 05:06 Zonespace27