Ruin0x11 / OpenNefia

(Archived) Moddable engine reimplementation of the Japanese roguelike Elona.
MIT License
116 stars 18 forks source link

Refactor locale functions to take full map object data instead of the result of `ILocalizable:produce_locale_data()` #271

Open Ruin0x11 opened 3 years ago

Ruin0x11 commented 3 years ago

I've been racking my brain over this one.

Basically, what the localization environment was supposed to be in my mind was a way of taking the bundle of localization-specific data for map objects like gender/amount and formatting it in a pure way. Map objects and the like would never get passed into the functions under locale/ because they didn't "belong" there. Instead, ILocalizable:produce_locale_data() would get called on them to retrieve just what would be needed for the functions inside the locale environment (he(), his_owned(), etc.).

What I didn't anticipate was that generating the name of an item is an expensive operation that requires the entire data of the item to be available in order to work. If you pass a default name for the item with its amount as 1 and later on need the name generated with its amount set to something else, you can't do that with :produce_locale_data(). You'd have to build the item's name from the code with :build_name() ahead of time. But it sometimes feels like that misses the point of :produce_locale_data() if we're doing all the work from the code anyway.

At the same time, we'd never have functions like he() or his_owned()outside the locale environment, because those are specific to a single language only.

I think the :produce_locale_data() thing works pretty well for characters, but only for characters. Maybe :produce_locale_data() for items should just call :build_name() on the item as a default, and if you need to build the item's name with a different amount, you'd just call :build_name() on it yourself. Then again, you wouldn't be able to pass the item's amount to the locale function in a single argument this way (a string gets returned from :produce_locale_data() instead of a table), and the item's amount is actually used for some functions in the locale environment.

What if, after designing the API so you need to pass the item's amount optionally, you end up needing the number of items for a new language because of some special pluralization rules or something specific to that language, but the item amount wasn't passed in the code?

Still, there wasn't originally any thought given to the localization code in HSP Elona at all, so maybe we'd just have to live with that weirdness between localizing characters and items, so long as it works.

Ruin0x11 commented 3 years ago

I think biting the dust on the "purity" argument and just passing the full character/item is the simplest option at this point. This would mean getting rid of ILocalizable. If someone wants to fire off a bunch of instance methods on such objects that mutate state inside a locale function, they'd be free to do so, but it would be a dumb idea.