H-uru / Plasma

Cyan Worlds's Plasma game engine
http://h-uru.github.io/Plasma/
GNU General Public License v3.0
205 stars 81 forks source link

Remove W-suffixed Python methods and always support Unicode properly #1310

Closed dgelessus closed 1 year ago

dgelessus commented 1 year ago

Working on removing the remaining non-Unicode-aware Python methods, which I previously bumped into in #1297. The following classes still have separate W and non-W methods:

I've only done "surface-level" testing for most of the changed script code. However, most calls touched by this change fall into one of the following categories that shouldn't need much testing:

As far as I can tell, only the following calls are actually interesting:

Feedback welcome on whether I should squash the commits further in the end. Keeping them separate makes it possible to see which calls were still using the non-W-prefixed methods (and were thus potentially not Unicode-aware). Squashing them together would eliminate some back-and-forth changes (making git blame a bit cleaner).

Hoikas commented 1 year ago

Feedback welcome on whether I should squash the commits further in the end. Keeping them separate makes it possible to see which calls were still using the non-W-prefixed methods (and were thus potentially not Unicode-aware). Squashing them together would eliminate some back-and-forth changes (making git blame a bit cleaner).

For now, I would say keep the transitional commits. I wouldn't worry about squashing anything until the work is complete, and, even then, it might be better to just keep the commits as they are. As long as your history tells a story that makes sense, I'm not picky either way.

dgelessus commented 1 year ago

Alright, that's all the easy obvious stuff taken care of. That is, all Python methods that had both W and non-W versions now have just one suffix-less version that is fully Unicode-aware. I've also deleted the old, non-Unicode-aware code from the C++ side of pfPython.

There's still more work to be done here, which I think would be better off in a separate PR, because it's mostly separate and might be less straightforward. Specifically:

dgelessus commented 1 year ago

As mentioned above, I've only done thorough tests for the few code paths that looked (previously) problematic to me. Everything else looked trivial to me while reading through the diffs. Let me know if you notice anything that I should re-test in particular.