Add hooks for modifying elements and thresholds. #127

pyrrhicPachyderm · 2023-10-08T07:02:07Z

To aid in implementing Dark Fire.

To avoid double-counting element markers in player areas for the aid board, each player area's countItems() loop counts non-token elements separately. This makes it difficult to implement a hook to modify elements, as both counting methods, countItems() and aidBoard's scanElements(), have to be handled separately. To make this easier, shift the onus for avoiding double-counting to the aid board, removing the need for tracking non-token elements. This likely comes with a performance penalty, as now scanElements() must call getZones() on each element token. But scanElements() runs only every two seconds, and getZones() isn't a raycast, so this shouldn't be too bad.

As more hooks are added, the colour's going to be needed more often, so for performance reasons, just find it once at the beginning of the function.

In case Dark Fire wants to delete the Moon or Fire element bag, leaving just a Dark Fire bag.

To avoid variable name shadowing. Instead of defining a shadowed local copy of the color variable in countItems() and other functions, simply edit the outer version of the variable.

iakona

so assuming the idea for darkfire is that you take an 8 element table and turn it into a 7 element table with fire and moon combined, you probably want to fix all the assumed hard coding that you'll always have 8 elements (since in theory something could add a 9th)

objects/aidBoard/script.lua

iakona · 2023-10-10T02:42:38Z

script.lua

@@ -5729,6 +5729,18 @@ function setupPlayerArea(params)
    function Elements:__tostring()
        return table.concat(self, "")
    end
+    local function modifyElements(pars)


for syntax purposes, i've typically been calling the parameter params

I did that initially, but on first push, the linter complained that the params argument in in modifyElements() was shadowing the params argument in the outer setupPlayerArea() function. modifyElements() has to be called inside setupPlayerArea(), 'cause the definition of Elements sits inside setupPlayerArea().

I'm not actually sure why Elements is defined inside setupPlayerArea(). If this definition could be moved outside setupPlayerArea(), then the definition of modifyElements() could likewise be moved outside, thus dodging any shadowing problems. I wonder if it would then also be possible to access the definition of Elements from the aidBoard script, 'cause I noticed that the aidBoard script contains a lot of lines of duplicated code to define Elements again.

So, do you know why Elements is defined inside setupPlayerArea()? Or a better way of avoiding the linter error?

i dont remember why elements is defined in there, Tom (the guy who wrote the python plugin to deconstruct and construct tts save files) was the one who wrote this. i imagine it can be moved outside

edit: test

I'll move it outside. Given it's duplicated in aidBoard, I might do something about trying to avoid that duplication, as well. I think I need to read up on metatables to know how to do that properly. I did notice that one of Tom's old pull requests (#48) mentioned that tts-tools can handle Lua modules, so I may also look into that as a possibility.

Ah, looks like Tom pulled support for modules in tts-tools (tomprince/tabletop-tools#21), so I won't be using a module for this.

is it not possible to move Elements to live outside of the current scope it's in while still having it exist in this file?

Oh, it's certainly possible, and I'm treating it as a backup option. But the definition of Elements is over 30 lines of code that's duplicated four times (in Global, aidBoard, PowerEditor and SpiritEditor, so it'd be really nice to get those consolidated. I'm still investigating whether that's possible.

Alas, I can't find any way this is possible. I've just moved the global copy of Elements outside of setupPlayerArea(), and not touched the other three copies.

pyrrhicPachyderm · 2023-10-10T05:37:12Z

Replacing the table with a 7 element table isn't quite the idea, 'cause that wouldn't work correctly with the aid board. The idea is to still return an 8 element table, but where the Moon and Fire entries are both equal to the sum of the input Moon and Fire entries. Altering all Shadows' thresholds in exactly the same way then makes threshold-checking work correctly.

The advantage of doing it that way is that it then makes the aid board work correctly, assuming you'd treat all your Moon as Fire for a Fire event, or vice versa.

The only place it'd turn 8 elements into 7 is for the actual element bags along the top of the player area. The second element bag would be a dark fire bag, and it'd still be treated as either the fire or the moon bag (it doesn't matter which). The other bag can be nilled out (hence me adding the nil check when editing the buttons that give the counts on the element bags), but the table of element counts stored in the code would still have the full 8 elements.

CUSTOM.md

iakona · 2023-10-15T20:49:59Z

script.lua

@@ -5729,6 +5729,18 @@ function setupPlayerArea(params)
    function Elements:__tostring()
        return table.concat(self, "")
    end
+    local function modifyElements(pars)


i dont remember why elements is defined in there, Tom (the guy who wrote the python plugin to deconstruct and construct tts save files) was the one who wrote this. i imagine it can be moved outside

edit: test

objects/aidBoard/script.lua

script.lua

This allows the related functions to use "params" as their argument without shadowing "params" from setupPlayerArea().

pyrrhicPachyderm · 2023-10-20T01:47:28Z

Merging the card gain hook has left this with a couple of merge conflicts. They're both pretty trivial to resolve (it's just that both have added new entries to the bottom of CUSTOM.md, and new tags in savegame.json), but I can rebase this branch onto the current beta branch if you'd prefer not to have to resolve them.

iakona · 2023-10-21T15:46:52Z

rebase or another commit is fine with me

script.lua

iakona · 2023-10-23T23:43:24Z

script.lua

+    for _,object in pairs(getObjectsWithTag("Modify Elements")) do
+        params.elements = object.call("modifyElements", params)
+    end
+    return Elements:new(params.elements)


do we need to call Elements:new again? we called it before to pass in parameter to modifyElements. is this just kind of sanity checking the return value of from the object function callss?

People writing their own modifyElements() functions won't have access to the Elements metatable (or more specifically, won't have access to the functions in the Elements metatable, even if they can access the metatable itself). And I haven't even mentioned the metatable in the documentation. That's no concern if they modify and return params.elements, 'cause then the metatable will still be attached. But if they decide to build the table they return from scratch (which the documentation doesn't tell them not to do), then it'll be a plain table without the elements metatable attached. Calling Elements:new() on it reattaches the metatable, making it essentially a proper Elements "object" again, rather than just a plain table.

And it's necessary to ensure that the object has the metatable attached, 'cause addThresholdDecals() later uses elements:threshold() from the metatable:

spirit-island-mod/script.lua

Line 5811 in 083daed

if elements:threshold(thresholdElements) then

makes sense, thanks

pyrrhicPachyderm added 7 commits October 8, 2023 16:31

Find the player colour at the begnning of countItems().

5bff598

As more hooks are added, the colour's going to be needed more often, so for performance reasons, just find it once at the beginning of the function.

Add hooks for modifying elements and thresholds.

a28d214

Add a nil check when editing element counts in countItems().

2c0e45e

In case Dark Fire wants to delete the Moon or Fire element bag, leaving just a Dark Fire bag.

Make functions in setupPlayerArea() local.

6f8cff8

Rename some variables to avoid shadowing.

be704cf

Change how the color variable is handled in setupPlayerArea().

8d3b24c

To avoid variable name shadowing. Instead of defining a shadowed local copy of the color variable in countItems() and other functions, simply edit the outer version of the variable.

iakona reviewed Oct 10, 2023

View reviewed changes

iakona reviewed Oct 15, 2023

View reviewed changes

script.lua Show resolved Hide resolved

pyrrhicPachyderm added 2 commits October 16, 2023 12:07

Describe the table of elements better in CUSTOM.md.

bb8c31d

Move Elements and related functions outside setupPlayerArea().

261e331

This allows the related functions to use "params" as their argument without shadowing "params" from setupPlayerArea().

Merge branch 'beta' into elements-hook

083daed

iakona reviewed Oct 22, 2023

View reviewed changes

script.lua Show resolved Hide resolved

iakona reviewed Oct 23, 2023

View reviewed changes

iakona merged commit c792f9e into spirit-island:beta Oct 24, 2023
2 checks passed

pyrrhicPachyderm deleted the elements-hook branch October 24, 2023 00:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add hooks for modifying elements and thresholds. #127

Add hooks for modifying elements and thresholds. #127

pyrrhicPachyderm commented Oct 8, 2023

iakona left a comment

iakona Oct 10, 2023

pyrrhicPachyderm Oct 10, 2023

iakona Oct 15, 2023 •

edited

Loading

pyrrhicPachyderm Oct 16, 2023

pyrrhicPachyderm Oct 16, 2023 •

edited

Loading

iakona Oct 16, 2023

pyrrhicPachyderm Oct 17, 2023

pyrrhicPachyderm Oct 17, 2023

pyrrhicPachyderm commented Oct 10, 2023 •

edited

Loading

iakona Oct 15, 2023 •

edited

Loading

pyrrhicPachyderm commented Oct 20, 2023 •

edited

Loading

iakona commented Oct 21, 2023

iakona Oct 23, 2023

pyrrhicPachyderm Oct 23, 2023

pyrrhicPachyderm Oct 23, 2023

iakona Oct 24, 2023

Add hooks for modifying elements and thresholds. #127

Add hooks for modifying elements and thresholds. #127

Conversation

pyrrhicPachyderm commented Oct 8, 2023

iakona left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iakona Oct 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pyrrhicPachyderm Oct 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pyrrhicPachyderm commented Oct 10, 2023 • edited Loading

iakona Oct 15, 2023 • edited Loading

Choose a reason for hiding this comment

pyrrhicPachyderm commented Oct 20, 2023 • edited Loading

iakona commented Oct 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iakona Oct 15, 2023 •

edited

Loading

pyrrhicPachyderm Oct 16, 2023 •

edited

Loading

pyrrhicPachyderm commented Oct 10, 2023 •

edited

Loading

iakona Oct 15, 2023 •

edited

Loading

pyrrhicPachyderm commented Oct 20, 2023 •

edited

Loading