Fixed repeats not working correctly

core/src/main/kotlin/com/willfp/libreforge/NamedValue.kt

@@ -1,8 +1,11 @@
 package com.willfp.libreforge
 
+import com.willfp.eco.core.placeholder.InjectablePlaceholder
 import com.willfp.eco.core.placeholder.StaticPlaceholder
+import com.willfp.eco.core.placeholder.context.PlaceholderContext
+import com.willfp.eco.core.placeholder.templates.SimpleInjectablePlaceholder
 
-class NamedValue constructor(
+open class NamedValue constructor(
     identifiers: Collection<String>,
     value: String
 ) {
@@ -16,13 +19,70 @@ class NamedValue constructor(
         value: Any
     ) : this(identifiers, value.toString())
 
-    val placeholders = identifiers.map {
+    open val placeholders: List<InjectablePlaceholder> = identifiers.map {
         StaticPlaceholder(
             it
         ) { value }
     }
 }
 
-fun Collection<NamedValue>.mapToPlaceholders(): Array<out StaticPlaceholder> {
+/*
+
+Ideally, this would be merged into NamedValue, but that would break backwards compatibility,
+because the () -> Any constructor would be ambiguous with the Any constructor, and casting
+doesn't seem to fix the problem either.
+
+This is internal because one: it's a hack, and two: there isn't really a case where this
+would be needed outside of repeats, which is a purely internal feature anyway.
+
+Because of how expression caching works, DynamicNamedValue also needs to provide placeholders
+that give different hashes each time, to force a re-calculation of the expression.
+
+ */
+
+internal class DynamicNumericValue(
+    identifiers: Collection<String>,
+    value: () -> Number
+) : NamedValue(identifiers, value()) {
+    constructor(
+        identifier: String,
+        value: () -> Number
+    ) : this(listOf(identifier), value)
+
+    override val placeholders: List<InjectablePlaceholder> = identifiers.map {
+        DynamicHashPlaceholder(
+            it,
+            value
+        )
+    }
+
+    private class DynamicHashPlaceholder(
+        private val identifier: String,
+        private val fn: () -> Number
+    ) : SimpleInjectablePlaceholder(identifier) {
+        override fun getValue(p0: String, p1: PlaceholderContext) = fn().toString()
+
+        override fun hashCode(): Int {
+            // Use the value of the function to force a re-calculation of the expression
+            return fn().toInt() * 31 + identifier.hashCode()
+        }
+
+        override fun equals(other: Any?): Boolean {
+            if (this === other) {
+                return true
+            }
+
+            if (other !is DynamicHashPlaceholder) {
+                return false
+            }
+
+            return other.identifier == this.identifier
+                    && other.fn() == this.fn()
+        }
+    }
+}
+
+
+fun Collection<NamedValue>.mapToPlaceholders(): Array<out InjectablePlaceholder> {
     return this.flatMap { it.placeholders }.toTypedArray()
 }

core/src/main/kotlin/com/willfp/libreforge/SeparatorAmbivalentConfig.kt

@@ -89,6 +89,12 @@ private class SeparatorAmbivalentConfig(
 
     override fun injectPlaceholders(vararg placeholders: InjectablePlaceholder) =
         config.injectPlaceholders(*placeholders)
+
+    override fun equals(other: Any?) =
+        config == other
+
+    override fun hashCode() =
+        config.hashCode()
 }
 
 fun Config.separatorAmbivalent(): Config =

core/src/main/kotlin/com/willfp/libreforge/effects/ElementLike.kt

@@ -2,6 +2,7 @@ package com.willfp.libreforge.effects
 
 import com.willfp.eco.core.config.interfaces.Config
 import com.willfp.eco.core.integrations.antigrief.AntigriefManager
+import com.willfp.libreforge.DynamicNumericValue
 import com.willfp.libreforge.NamedValue
 import com.willfp.libreforge.conditions.ConditionList
 import com.willfp.libreforge.effects.arguments.EffectArgumentList
@@ -49,6 +50,10 @@ abstract class ElementLike {
             return doTrigger(trigger)
         }
 
+        // Extra initial injection, otherwise it's not possible to use injections
+        // in the repeat configs.
+        config.addInjectablePlaceholder(trigger.placeholders)
+
         // It would be nice to abstract repeat/delay away here, but that would be
         // really, really, overengineering it - even for me.
         val repeatTimes = config.getIntFromExpression("repeat.times", trigger.data).coerceAtLeast(1)
@@ -59,7 +64,7 @@ abstract class ElementLike {
         trigger.addPlaceholder(NamedValue("repeat_times", repeatTimes))
         trigger.addPlaceholder(NamedValue("repeat_start", repeatStart))
         trigger.addPlaceholder(NamedValue("repeat_increment", repeatIncrement))
-        trigger.addPlaceholder(NamedValue("repeat_count", repeatCount))
+        trigger.addPlaceholder(DynamicNumericValue("repeat_count") { repeatCount })
 
         val delay = config.getIntFromExpression("delay", trigger.data)
             .coerceAtLeast(0)
@@ -123,15 +128,15 @@ abstract class ElementLike {
                     )
                 )
             ) true else didTrigger
+
+            repeatCount += repeatIncrement
         }
 
         // Can't delay initial execution for things that modify events.
         if (delay == 0L) {
             repeat(repeatTimes) {
                 trigger()
             }
-
-            repeatCount += repeatIncrement
         } else {
             // Delay between each repeat.
             var repeats = 0

core/src/main/kotlin/com/willfp/libreforge/triggers/DispatchedTrigger.kt

@@ -1,6 +1,6 @@
 package com.willfp.libreforge.triggers
 
-import com.willfp.eco.core.placeholder.StaticPlaceholder
+import com.willfp.eco.core.placeholder.InjectablePlaceholder
 import com.willfp.libreforge.NamedValue
 import org.bukkit.entity.Player
 
@@ -11,7 +11,7 @@ data class DispatchedTrigger(
 ) {
     private val _placeholders = mutableListOf<NamedValue>()
 
-    val placeholders: List<StaticPlaceholder>
+    val placeholders: List<InjectablePlaceholder>
         get() = _placeholders.flatMap { it.placeholders }
 
     fun addPlaceholder(placeholder: NamedValue) {