Invalidate remotes on chart being deserialized

Also fixed memory leak when removing remotes

src/main/kotlin/ru/dbotthepony/mc/otm/container/CombinedContainer.kt

@@ -10,20 +10,16 @@ import it.unimi.dsi.fastutil.objects.Object2ObjectFunction
 import it.unimi.dsi.fastutil.objects.Reference2ObjectOpenHashMap
 import net.minecraft.world.Container
 import net.minecraft.world.entity.player.Player
-import net.minecraft.world.inventory.Slot
 import net.minecraft.world.item.Item
 import net.minecraft.world.item.ItemStack
-import ru.dbotthepony.mc.otm.container.util.IContainerSlot
 import ru.dbotthepony.mc.otm.container.util.containerSlot
 import ru.dbotthepony.mc.otm.container.util.iterator
-import ru.dbotthepony.mc.otm.container.util.slotIterator
 import ru.dbotthepony.mc.otm.core.collect.concatIterators
 import ru.dbotthepony.mc.otm.core.collect.filter
 import ru.dbotthepony.mc.otm.core.collect.flatMap
 import ru.dbotthepony.mc.otm.core.collect.map
 import ru.dbotthepony.mc.otm.core.isNotEmpty
 import ru.dbotthepony.mc.otm.core.stream
-import java.util.LinkedList
 import java.util.stream.Stream
 
 class CombinedContainer(containers: Stream<Pair<Container, Iterable<Int>>>) : IMatteryContainer {
@@ -33,18 +29,6 @@ class CombinedContainer(containers: Stream<Pair<Container, Iterable<Int>>>) : IM
 	private inner class Slot(override val slot: Int, val outer: IContainerSlot) : IContainerSlot by outer {
 		override val container: Container
 			get() = this@CombinedContainer
-
-		override fun setChanged() {
-			outer.setChanged()
-		}
-
-		override fun component1(): Int {
-			return super.component1()
-		}
-
-		override fun component2(): ItemStack {
-			return super.component2()
-		}
 	}
 
 	private val slots: ImmutableList<Slot>

src/main/kotlin/ru/dbotthepony/mc/otm/container/ContainerHelpers.kt

@@ -18,7 +18,6 @@ import net.minecraft.world.item.ItemStack
 import net.minecraft.world.item.enchantment.EnchantmentEffectComponents
 import net.minecraft.world.item.enchantment.EnchantmentHelper
 import net.neoforged.neoforge.fluids.capability.IFluidHandler
-import ru.dbotthepony.mc.otm.container.util.IContainerSlot
 import ru.dbotthepony.mc.otm.container.util.ItemStackHashStrategy
 import ru.dbotthepony.mc.otm.container.util.containerSlot
 import ru.dbotthepony.mc.otm.container.util.slotIterator

src/main/kotlin/ru/dbotthepony/mc/otm/container/IContainerSlot.kt

@@ -0,0 +1,68 @@
+package ru.dbotthepony.mc.otm.container
+
+import net.minecraft.world.Container
+import net.minecraft.world.item.Item
+import net.minecraft.world.item.ItemStack
+import net.minecraft.world.item.Items
+import ru.dbotthepony.kommons.util.Delegate
+import ru.dbotthepony.mc.otm.core.isNotEmpty
+
+/**
+ * While this somewhat similar to [net.minecraft.world.inventory.Slot], this slot is not meant
+ * for Player interaction.
+ */
+interface IContainerSlot : Delegate<ItemStack> {
+	val slot: Int
+	val container: Container
+
+	operator fun component1() = slot
+	operator fun component2() = item
+
+	fun getMaxStackSize(item: ItemStack = this.item): Int {
+		return container.maxStackSize
+	}
+
+	val isForbiddenForAutomation: Boolean get() {
+		return getFilter() === Items.AIR
+	}
+
+	fun getFilter(): Item?
+
+	/**
+	 * @return whenever the filter was set. Returns false only if container can't be filtered.
+	 */
+	fun setFilter(filter: Item? = null): Boolean
+
+	val hasFilter: Boolean
+		get() = getFilter() != null
+
+	fun remove() {
+		container[slot] = ItemStack.EMPTY
+	}
+
+	fun remove(count: Int): ItemStack {
+		return container.removeItem(slot, count)
+	}
+
+	override fun get(): ItemStack {
+		return container[slot]
+	}
+
+	override fun accept(t: ItemStack) {
+		container[slot] = t
+	}
+
+	fun setChanged() {
+		container.setChanged()
+	}
+
+	var item: ItemStack
+		get() = container[slot]
+		set(value) { container[slot] = value }
+
+	val isEmpty: Boolean
+		get() = container[slot].isEmpty
+
+	val isNotEmpty: Boolean
+		get() = container[slot].isNotEmpty
+}

src/main/kotlin/ru/dbotthepony/mc/otm/container/IMatteryContainer.kt

@@ -5,7 +5,6 @@ import net.minecraft.world.item.Item
 import net.minecraft.world.item.ItemStack
 import net.minecraft.world.item.Items
 import net.minecraft.world.item.crafting.RecipeInput
-import ru.dbotthepony.mc.otm.container.util.IContainerSlot
 import ru.dbotthepony.mc.otm.core.collect.filter
 import ru.dbotthepony.mc.otm.core.collect.map
 import ru.dbotthepony.mc.otm.core.isNotEmpty

src/main/kotlin/ru/dbotthepony/mc/otm/container/MatteryContainer.kt

@@ -22,7 +22,6 @@ import net.minecraft.world.item.Items
 import net.neoforged.neoforge.common.util.INBTSerializable
 import org.apache.logging.log4j.LogManager
 import ru.dbotthepony.kommons.util.Delegate
-import ru.dbotthepony.mc.otm.container.util.IContainerSlot
 import ru.dbotthepony.mc.otm.core.addSorted
 import ru.dbotthepony.mc.otm.core.collect.any
 import ru.dbotthepony.mc.otm.core.collect.count

src/main/kotlin/ru/dbotthepony/mc/otm/container/util/Iterators.kt

@@ -3,76 +3,14 @@ package ru.dbotthepony.mc.otm.container.util
 import net.minecraft.world.Container
 import net.minecraft.world.item.Item
 import net.minecraft.world.item.ItemStack
-import net.minecraft.world.item.Items
-import ru.dbotthepony.kommons.util.Delegate
+import ru.dbotthepony.mc.otm.container.IContainerSlot
 import ru.dbotthepony.mc.otm.container.IMatteryContainer
 import ru.dbotthepony.mc.otm.container.get
-import ru.dbotthepony.mc.otm.container.set
 import ru.dbotthepony.mc.otm.core.collect.filter
 import ru.dbotthepony.mc.otm.core.collect.map
 import ru.dbotthepony.mc.otm.core.isNotEmpty
 
-/**
- * While this somewhat similar to [net.minecraft.world.inventory.Slot], this slot is not meant
- * for Player interaction.
- */
-interface IContainerSlot : Delegate<ItemStack> {
-	val slot: Int
-	val container: Container
-
-	operator fun component1() = slot
-	operator fun component2() = item
-
-	fun getMaxStackSize(item: ItemStack = this.item): Int {
-		return container.maxStackSize
-	}
-
-	val isForbiddenForAutomation: Boolean get() {
-		return getFilter() === Items.AIR
-	}
-
-	fun getFilter(): Item?
-
-	/**
-	 * @return whenever the filter was set. Returns false only if container can't be filtered.
-	 */
-	fun setFilter(filter: Item? = null): Boolean
-
-	val hasFilter: Boolean
-		get() = getFilter() != null
-
-	fun remove() {
-		container[slot] = ItemStack.EMPTY
-	}
-
-	fun remove(count: Int): ItemStack {
-		return container.removeItem(slot, count)
-	}
-
-	override fun get(): ItemStack {
-		return container[slot]
-	}
-
-	override fun accept(t: ItemStack) {
-		container[slot] = t
-	}
-
-	fun setChanged() {
-		container.setChanged()
-	}
-
-	var item: ItemStack
-		get() = container[slot]
-		set(value) { container[slot] = value }
-
-	val isEmpty: Boolean
-		get() = container[slot].isEmpty
-
-	val isNotEmpty: Boolean
-		get() = container[slot].isNotEmpty
-}
-
-class ContainerSlot(override val slot: Int, override val container: Container) : IContainerSlot {
+class SimpleContainerSlot(override val slot: Int, override val container: Container) : IContainerSlot {
 	init {
 		require(slot in 0 until container.containerSize) { "Slot out of bounds: $slot (container: $container with size ${container.containerSize})" }
 	}
@@ -90,7 +28,7 @@ fun Container.containerSlot(slot: Int): IContainerSlot {
 	if (this is IMatteryContainer) {
 		return containerSlot(slot)
 	} else {
-		return ContainerSlot(slot, this)
+		return SimpleContainerSlot(slot, this)
 	}
 }
 
@@ -110,8 +48,8 @@ fun Container.slotIterator(nonEmpty: Boolean = true): Iterator<IContainerSlot> {
 	if (this is IMatteryContainer) {
 		return slotIterator(nonEmpty)
 	} else if (nonEmpty) {
-		return (0 until containerSize).iterator().filter { this[it].isNotEmpty }.map { ContainerSlot(it, this) }
+		return (0 until containerSize).iterator().filter { this[it].isNotEmpty }.map { SimpleContainerSlot(it, this) }
 	} else {
-		return (0 until containerSize).iterator().map { ContainerSlot(it, this) }
+		return (0 until containerSize).iterator().map { SimpleContainerSlot(it, this) }
 	}
 }

src/main/kotlin/ru/dbotthepony/mc/otm/core/chart/AbstractHistoryChart.kt

@@ -177,6 +177,7 @@ abstract class AbstractHistoryChart<V : Any>(
 
 		this.accumulator.addAll(accumulator)
 		this.values.addAll(values)
+		this.remotes.forEach { it.invalidate() }
 	}
 
 	fun clear() {

src/main/kotlin/ru/dbotthepony/mc/otm/menu/MatteryMenu.kt

@@ -8,6 +8,7 @@ import it.unimi.dsi.fastutil.ints.IntCollection
 import it.unimi.dsi.fastutil.objects.Reference2ObjectFunction
 import it.unimi.dsi.fastutil.objects.Reference2ObjectOpenHashMap
 import it.unimi.dsi.fastutil.objects.ReferenceArrayList
+import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet
 import net.minecraft.network.FriendlyByteBuf
 import net.minecraft.network.RegistryFriendlyByteBuf
 import net.minecraft.network.protocol.common.custom.CustomPacketPayload
@@ -385,11 +386,11 @@ abstract class MatteryMenu(
 
 	private val externalSlots = ConditionalSet<Slot>()
 	private val quickMoveMapping = Reference2ObjectOpenHashMap<Slot, ReferenceArrayList<Collection<Slot>>>()
+	private val seenSlots = ReferenceOpenHashSet<Slot>()
 
 	override fun addSlot(pSlot: Slot): Slot {
-		if (pSlot in slots) {
+		if (!seenSlots.add(pSlot))
 			return pSlot
-		}
 
 		if (pSlot is UserFilteredSlot && !pSlot.hasSetFilter) {
 			val container = pSlot.container

src/main/kotlin/ru/dbotthepony/mc/otm/network/syncher/DynamicSynchableGroup.kt

@@ -180,7 +180,7 @@ class DynamicSynchableGroup<T : ISynchable>(
 
 	private data class Slot<T : Any>(val synchable: T, val id: Int)
 
-	private val remoteStates = CopyOnWriteArrayList<RemoteState>()
+	private val remoteStates = ArrayList<RemoteState>()
 	private val value2slot = HashMap<T, Slot<T>>()
 	private val id2slot = Int2ObjectOpenHashMap<Slot<T>>()
 	private val idAllocator = IDAllocator()
@@ -271,7 +271,7 @@ class DynamicSynchableGroup<T : ISynchable>(
 			value2slot.clear()
 			id2slot.clear()
 
-			remoteStates.forEach { it.clear() }
+			remoteStates.toTypedArray().forEach { it.clear() }
 		}
 	}
 

src/main/kotlin/ru/dbotthepony/mc/otm/network/syncher/SynchableGroup.kt

@@ -40,6 +40,7 @@ import java.util.function.DoubleSupplier
 import java.util.function.IntSupplier
 import java.util.function.LongSupplier
 import java.util.function.Supplier
+import kotlin.collections.ArrayList
 import kotlin.concurrent.withLock
 
 /**
@@ -56,7 +57,6 @@ import kotlin.concurrent.withLock
  */
 @Suppress("UNUSED")
 class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
-	private val lock = ReentrantLock()
 	private val slots = ArrayList<Slot?>()
 	private val gaps = IntAVLTreeSet()
 	private val observers = ArrayList<Slot>()
@@ -109,7 +109,7 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 		val id: Int
 
 		private var isRemoved = false
-		val remoteSlots = CopyOnWriteArrayList<Remote.RemoteSlot>()
+		val remoteSlots = ArrayList<Remote.RemoteSlot>()
 
 		override fun close() {
 			if (!isRemoved) {
@@ -118,7 +118,7 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 				gaps.add(id)
 
 				observers.remove(this)
-				remoteSlots.forEach { it.remove() }
+				remoteSlots.toTypedArray().forEach { it.remove() }
 			}
 		}
 
@@ -128,32 +128,28 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 
 		private fun markDirty() {
 			if (!isRemote && !isRemoved) {
-				remoteSlots.forEach {
-					it.markDirty()
-				}
+				remoteSlots.forEach { it.markDirty() }
 			}
 		}
 
 		init {
-			lock.withLock {
-				if (synchable.shouldBeObserved) {
-					observers.add(this)
-				}
+			if (synchable.shouldBeObserved) {
+				observers.add(this)
+			}
 
-				if (gaps.isNotEmpty()) {
-					val gap = gaps.firstInt()
-					gaps.remove(gap)
-					id = gap
-					check(slots[id] == null) { "Expected slot $id to be empty" }
-					slots[id] = this
-				} else {
-					id = slots.size
-					slots.add(this)
-				}
+			if (gaps.isNotEmpty()) {
+				val gap = gaps.firstInt()
+				gaps.remove(gap)
+				id = gap
+				check(slots[id] == null) { "Expected slot $id to be empty" }
+				slots[id] = this
+			} else {
+				id = slots.size
+				slots.add(this)
+			}
 
-				remotes.forEach {
-					it.addSlot(this)
-				}
+			remotes.forEach {
+				it.addSlot(this)
 			}
 		}
 	}
@@ -301,12 +297,11 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 	 * To get changes to be networked to remote client, [write] should be called.
 	 */
 	inner class Remote(private val listener: Runnable) : Closeable, IRemoteState {
-		constructor() : this(Runnable {  })
+		constructor() : this(Runnable {})
 
-		@Volatile
 		private var isRemoved = false
 		internal val dirty = ConcurrentLinkedQueue<RemoteSlot>()
-		private val remoteSlots = CopyOnWriteArrayList<RemoteSlot>()
+		private val remoteSlots = ArrayList<RemoteSlot>()
 
 		internal inner class RemoteSlot(val slot: Slot) {
 			private val isDirty = AtomicBoolean()
@@ -357,6 +352,7 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 				remoteSlots.remove(this)
 				dirty.remove(this)
 				cleanable.clean()
+				slot.remoteSlots.remove(this)
 			}
 
 			override fun hashCode(): Int {
@@ -369,15 +365,13 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 		}
 
 		init {
-			lock.withLock {
-				slots.forEach {
-					if (it != null) {
-						addSlot(it)
-					}
+			slots.forEach {
+				if (it != null) {
+					addSlot(it)
 				}
-
-				remotes.add(this)
 			}
+
+			remotes.add(this)
 		}
 
 		internal fun addSlot(slot: Slot) {
@@ -435,15 +429,13 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 			if (!isRemoved) {
 				isRemoved = true
 
-				lock.withLock {
-					remoteSlots.forEach {
-						it.remove()
-						it.slot.remoteSlots.remove(it)
-					}
-
-					remotes.remove(this)
-					dirty.clear()
+				remoteSlots.toTypedArray().forEach {
+					it.remove()
+					it.slot.remoteSlots.remove(it)
 				}
+
+				remotes.remove(this)
+				dirty.clear()
 			}
 		}
 	}