Make FString::LastIndexOf work like Javascript's equivalent

[Talon1024]

Why? FString::LastIndexOf is weird. For example, if you run this ZScript code:

Code: Select all

class StringTestEventHandler : EventHandler {
	override void PlayerEntered(PlayerEvent e) {
		static const String testStrings[] = { "hello", "hello.obj", "hello.obj.world", "hello.obj.obj.obj", "hello.obj.obj.obj.world" };

		for (int i = 0; i < 5; i++) {
			String test = testStrings[i];
			Console.Printf("Test string: \"%s\"\n", test);
			Console.Printf("LastIndexOf(\".obj\"): %d\n", test.LastIndexOf(".obj"));
			Console.Printf("LastIndexOf(\".\"): %d\n", test.LastIndexOf("."));
			Console.Printf("LastIndexOf(\"eeee\"): %d\n", test.LastIndexOf("eeee"));
			Console.Printf("LastIndexOf(\"eeeeeeeeeeeeeeeeeeeeeeeeeeeeee\"): %d\n", test.LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"));
		}
	}
}

The results from the current master branch are:

Code: Select all

Test string: "hello"
LastIndexOf(".obj"): -1
LastIndexOf("."): -1
LastIndexOf("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

Test string: "hello.obj"
LastIndexOf(".obj"): 8
LastIndexOf("."): 5
LastIndexOf("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

Test string: "hello.obj.world"
LastIndexOf(".obj"): 8
LastIndexOf("."): 9
LastIndexOf("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

Test string: "hello.obj.obj.obj"
LastIndexOf(".obj"): 16
LastIndexOf("."): 13
LastIndexOf("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

Test string: "hello.obj.obj.obj.world"
LastIndexOf(".obj"): 16
LastIndexOf("."): 17
LastIndexOf("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

I'm used to the way String.lastIndexOf works in JavaScript, and I'm sure most other developers are as well. The results from this code submission are:

Code: Select all

Test string: "hello"
LastIndexOf(".obj"): -1
LastIndexOf("."): -1
LastIndexOf("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

Test string: "hello.obj"
LastIndexOf(".obj"): 5
LastIndexOf("."): 5
LastIndexOf("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

Test string: "hello.obj.world"
LastIndexOf(".obj"): 5
LastIndexOf("."): 9
LastIndexOf("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

Test string: "hello.obj.obj.obj"
LastIndexOf(".obj"): 13
LastIndexOf("."): 13
LastIndexOf("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

Test string: "hello.obj.obj.obj.world"
LastIndexOf(".obj"): 13
LastIndexOf("."): 17
LastIndexOf("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

I just want to make sure I'm not breaking anything by making these changes.

Pull request: https://github.com/coelckers/gzdoom/pull/555

[phantombeta]

This will very likely break any mods that expect the current behaviour.

[Graf Zahl]

Correct. While this function is clearly broken thanks to the underlying implementation, just altering it is not acceptable. The fixed version needs to be a new function with a different name and the old one needs to be deprecated for coming ZScript versions.

[Talon1024]

OK, I added a new "IndexOfLast" function to FString in order to maintain backwards compatibility. Here are the results:

Code: Select all

Test string: "hello"
LastIndexOf(".obj"): -1
IndexOfLast(".obj"): -1
LastIndexOf("."): -1
IndexOfLast("."): -1
LastIndexOf("eeee"): -1
IndexOfLast("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1
IndexOfLast("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1
Test string: "hello.obj"
LastIndexOf(".obj"): 8
IndexOfLast(".obj"): 5
LastIndexOf("."): 5
IndexOfLast("."): 5
LastIndexOf("eeee"): -1
IndexOfLast("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1
IndexOfLast("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1
Test string: "hello.obj.world"
LastIndexOf(".obj"): 8
IndexOfLast(".obj"): 5
LastIndexOf("."): 9
IndexOfLast("."): 9
LastIndexOf("eeee"): -1
IndexOfLast("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1
IndexOfLast("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1
Test string: "hello.obj.obj.obj"
LastIndexOf(".obj"): 16
IndexOfLast(".obj"): 13
LastIndexOf("."): 13
IndexOfLast("."): 13
LastIndexOf("eeee"): -1
IndexOfLast("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1
IndexOfLast("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1
Test string: "hello.obj.obj.obj.world"
LastIndexOf(".obj"): 16
IndexOfLast(".obj"): 13
LastIndexOf("."): 17
IndexOfLast("."): 17
LastIndexOf("eeee"): -1
IndexOfLast("eeee"): -1
LastIndexOf("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1
IndexOfLast("eeeeeeeeeeeeeeeeeeeeeeeeeeeeee"): -1

[Blue Shadow]

This should be closed, since it's been added.