Endless Dream custom skin properties

[Catizard]

Motivation

This pr is a draft to implement the endless dream's only skin properties without introducing the conflicts with upstream. The original idea was proposed by @Arctice

Drop-in compatibility

This pr implements a "dynamic id system" to avoid producing any conflicts with the old skin system and not giving pressure to skin producers. To use it, skin writer needs to call the function exposed on main_state to get the id and
use it as usual. For example, to use the rival name in a text, we need to write something like table.insert(skin.text, { id = "rival", ref = 2 }) at old skin system. In endless dream's new skin system we only need to change the magic number 2 to a function call as table.insert(skin.text, {id = "rival", ref = main_state.rival() }).
(Note: This is only an example, we didn't expose the existed skin properties as a function to user)

Also this pr implements a "legacy mode" to allow skin makers to test their skins are working in vanilla raja or not.

Example

Currently, this pr contains an example for exposing the current bgm/sound directory, which isn't being exposed in skin properties and requested by #137. Here's a demo:

The skin used in the demo picture is Project-Shoko, code changes are:

skin.text = {
{id = "table", font = 4, size = -58, ref = 1003, overflow = 1, align = 1, shadowOffsetX = subShadowOffset, shadowOffsetY = subShadowOffset},
{id = "genre", font = 4, size = -58, ref = 13, overflow = 1, align = 1},
{id = "title", font = 3, size = -118, ref = 10, overflow = 1, align = 1},
{id = "fulltitle", font = 3, size = -118, ref = 12, overflow = 1, align = 1},
{id = "subtitle", font = 4, size = -58, ref = 11, overflow = 1, align = 1, shadowOffsetX = subShadowOffset, shadowOffsetY = subShadowOffset},
{id = "artist", font = 4, size = -58, ref = 14, overflow = 1, align = 1},
}

-- Endless Dream client
-- You can replace constantText with ref here if you don't want the prefix, the example here is want to show a more complex usage
if main_state.is_endlessdream ~= nil then
  local bgm_path = "BGM: " .. main_state.text(main_state.currentBGMPath());
  local sound_path = "Sound: " .. main_state.text(main_state.currentSoundPath());
  table.insert(skin.text, {
    id = "current_bgm_path", font = 4, size = -34, constantText = bgm_path, overflow = 0, align = 0
  });
  table.insert(skin.text, {
    id = "current_sound_path", font = 4, size = -34, constantText = sound_path, overflow = 0, align = 0
  });
end

and

-- Current bgm path
table.insert(skin.destination, {
  id = "current_bgm_path", loop = -1, dst = {
    {time = 0, x = 0, y = 50, w = 1720, h = 58, a = 00},
	{time = 300, x = 0, y = 50, a = 255},
	{time = 2750, x = 0, y = 50, a = 255},
	{time = 2950, x = 0, y = 50, a = 0},
  },
});
-- Current sound path 
table.insert(skin.destination, {
  id = "current_sound_path", loop = -1, dst = {
    {time = 0, x = 0, y = 100, w = 1720, h = 58, a = 00},
	{time = 300, x = 0, y = 100, a = 255},
	{time = 2750, x = 0, y = 100, a = 255},
	{time = 2950, x = 0, y = 100, a = 0},
  },
})

TODOs

• Bring the basic implementation of EventProperty, TimerProperty
• There's an ImageIndexProperty inside IntegerPropertyFactory, code. Currently I have no idea what this property is used for.
• Add two api for skin makers: (1) Query current ED's version; (2) Query a property's presence. To allow skin makers ensure their skin can work on different versions of ED

[Arctice]

I appreciate the effort but I think it's not going to be this simple.

This suggested check method doesn't meet our requirements. A skin that attempts to call main_state.is_endlessdream() will error out on upstream beatoraja with attempt to call a nil value. That's not the backwards compatibility we want. The point was to allow creating skins that can properly detect which version of beatoraja they're running on, and work with both endless dream and vanilla with no modifications.

Instead, I would like to suggest this kind of approach:
main_state.is_endlessdream ~= nil
This works even with this PR's code with no modifications; in addition, is_endlessdream doesn't even need to be a function, it may be any non-nil value such as a boolean or a string.

The second issue is that inserting lua functions that recover a value from the runtime, while clever, isn't going to work with dynamically changing values; I believe this implementation will only set them once during skin load. Instead, we will need to provide a function that recovers a numeric property id, the value of which will be dynamically assigned by us at skin load time - this will allow us to maintain full compatibility even with possible future upstream updates, because we won't ever have to worry about id collisions.

Along with this feature I think we should also add a skin configuration toggle to force compatibility mode - ie. allow loading a skin in "legacy" mode, with endless dream variables and properties disabled, to make it possible for skin authors to test both versions without having to juggle separate runtimes.

[Catizard]

This suggested check method doesn't meet our requirements. A skin that attempts to call main_state.is_endlessdream() will error out on upstream beatoraja with attempt to call a nil value. That's not the backwards compatibility we want. The point was to allow creating skins that can properly detect which version of beatoraja they're running on, and work with both endless dream and vanilla with no modifications.

Instead, I would like to suggest this kind of approach: main_state.is_endlessdream ~= nil This works even with this PR's code with no modifications; in addition, is_endlessdream doesn't even need to be a function, it may be any non-nil value such as a boolean or a string.

Thanks for pointing out, I completely forgot the field existence check. Example and code have been updated

The second issue is that inserting lua functions that recover a value from the runtime, while clever, isn't going to work with dynamically changing values; I believe this implementation will only set them once during skin load. Instead, we will need to provide a function that recovers a numeric property id, the value of which will be dynamically assigned by us at skin load time - this will allow us to maintain full compatibility even with possible future upstream updates, because we won't ever have to worry about id collisions.

I have a quick test and can confirm this is true, I'll update the pr implementation in near future. We might have some trouble at finding the unused id. Maybe writing a simple function to collect used id from StringPropertyFactory, NumberPropertyFactory etc......Need some time to investigate it.

Along with this feature I think we should also add a skin configuration toggle to force compatibility mode - ie. allow loading a skin in "legacy" mode, with endless dream variables and properties disabled, to make it possible for skin authors to test both versions without having to juggle separate runtimes.

I agree on this. Currently the only idea I can think of is to add a parameter to main function. I'll update the code later and see if we can find any clever solution or not.

[Catizard]

Updated the dynamic id allocation draft and example. Also added some explanation in the first pr comment.
Currently only StringProperty and FloatProperty is exposed because other properties are complicated. For example, there's a strange property type named RateProperty in FloatProperty, and DrawProperty, SongProperty, etc in BooleanProperty. Because they're separated everywhere they have a high possibility to break the dynamic id system :|

We still need some time to investigate what they're used for.

[Catizard]

Changed the base from main to 0.3.1 because this branch is based off 0.3.1 and currently it has too much other changes that do not belong to this pr compared to main. This pr is still a draft that possibly won't be merged during 0.3.1 stage.

[Catizard]

See the first pr message for todos!!!

Update:

• Add legacy skin mode in skin menu, which allows user test their skin's behavior in normal beatoraja environment
• I have went through all the skin property types to check if there could be some any collisions, see below
• Add custom integer property

Dynamic id allocation vs mystery raja code:

• Boolean:
  • It first goes through all BooleanType.values(): here, ED's custom boolean type property's id is starting from BooleanType so no collision here
  • Secondly, it checks if the property is between OPTION_COURSE_STAGE1 to OPTION_COURSE_STAGE4 or equals to OPTION_COURSE_STAGE_FINAL, which is around 280 to 290 and clearly there's no collision
  • Thirdly, if there's still no property was found, it'll call getBooleanProperty0, which is probably an old implementation. The ids this function uses are [1008, 1030, 1031, 5, 624, 625, 100, 290, 60, 61, 62]. The maximum value here is 1031, and our dynamic id would start from 2246 currently. So there's no possible to have a collision too.
• String
  • There's no extra code inside getStringProperty so there's definitely no collisions.
• Float
  • It first goes through all FloatTypeValues, like BooleanType our custom float type property's id is calculated from it so there's no collision.
  • If no property was found, it tries to iterate through RateTypeValues, which is probably an old FloatType implementation. Its id range is from 1 to 147. And our custom id is starting from 1115, clearly no collisions.
• Integer
  • Firstly, there're a bunch of if-else statements. By reading the code we can find that the ids here referenced are 1312~1327, [74, 106], 80~89, 110~114, 410~419, 280~289. So the maximum id here is 1327. And the maximum id value from IntegerPropertyFactory.ValueType is 1163 and our dynamic id would start from 1164. Clearly there's a collision. Therefore we have to modify the dynamic id start point for custom integer properties. See code for more details.
  • Secondly, it goes through the ValueType.values
  • If no property was found, it calls getIntegerProperty0, which is a big switch-case statement again. The id referenced here are [420, 421, 422, 426, 427, 96, 45, 46, 47, 48, 49, 100, 72, 154, 102, 103, 115, 155, 116, 156, 183, 184, 121, 151, 271, 122, 135, 157, 123, 136, 158, 152, 172, 108, 128, 153, 423, 424, 425, 10, 310, 311, 300, 14, 316, 314, 315, 107, 407, 75, 105, 370, 371, 71, 101, 171, 150, 170, 176, 178, 173, 175, 76, 177, 180, 200], none of them are close to either 1164 our 1327
  • Therefore, by setting dynamic id start point as 1328 we'll have no collisions from upstream.