Skip to content

Commit a979a1d

Browse files
SettingDustSettingDust
committed
fix(common): ConcurrentModificationException when generating mod sets config screen
fix #6 The kotlin flow emit won't waiting for subscribers collected. Kotlin/kotlinx.coroutines#2603 So, switching to `onEach` so that the loading is complete when emitted
1 parent 0baa2b1 commit a979a1d

File tree

12 files changed

+108
-76
lines changed

12 files changed

+108
-76
lines changed

‎build.gradle.kts

+2-3
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ plugins {
2525
// alias(libs.plugins.fabric.loom) apply false
2626

2727
alias(libs.plugins.shadow) apply false
28-
alias(libs.plugins.minotaur)
28+
alias(libs.plugins.minotaur) apply false
2929
alias(libs.plugins.cursegradle)
3030
}
3131

@@ -143,9 +143,8 @@ val fabricIntermediaryJar by tasks.registering(Jar::class) {
143143
from(zipTree(project(":fabric").tasks.named("remapJar").get().outputs.files.first()))
144144
from(zipTree(project(":quilt").tasks.named("remapJar").get().outputs.files.first()))
145145

146-
archiveBaseName.set(archives_name)
146+
archiveBaseName.set("$archives_name-fabric-intermediary")
147147
archiveVersion.set("${project.version}")
148-
archiveClassifier.set("fabric-intermediary")
149148

150149
duplicatesStrategy = DuplicatesStrategy.EXCLUDE
151150
}

‎common/src/game/kotlin/settingdust/modsets/game/ModSetsModMenu.kt

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ package settingdust.modsets.game
22

33
import com.terraformersmc.modmenu.api.ConfigScreenFactory
44
import com.terraformersmc.modmenu.api.ModMenuApi
5+
import settingdust.modsets.ModSets
56

67
class ModSetsModMenu : ModMenuApi {
78
override fun getModConfigScreenFactory() = ConfigScreenFactory {
8-
Rules.createScreen(it)
9+
ModSets.rules.createScreen(it)
910
}
1011
}

‎common/src/game/kotlin/settingdust/modsets/game/Rule.kt

+18-16
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import net.minecraft.network.chat.Component
1414
import net.minecraft.network.chat.HoverEvent
1515
import net.minecraft.network.chat.Style
1616
import settingdust.modsets.ModSets
17-
import settingdust.modsets.ModSetsConfig
17+
import settingdust.modsets.config
1818
import settingdust.modsets.game.Rules.getOrThrow
1919

2020
interface Described {
@@ -74,15 +74,15 @@ object LabelRule : OptionRule<Component> {
7474

7575
private val String.booleanBinding: Binding<Boolean>
7676
get() {
77-
val mods = Rules.modSets.getOrThrow(this).mods.toSet()
77+
val mods = ModSets.rules.modSets.getOrThrow(this).mods.toSet()
7878
return Binding.generic(
7979
true,
80-
{ mods.any { it !in ModSetsConfig.disabledMods } },
80+
{ mods.any { it !in ModSets.config.disabledMods } },
8181
{
8282
if (it) {
83-
ModSetsConfig.disabledMods.removeAll(mods)
83+
ModSets.config.disabledMods.removeAll(mods)
8484
} else {
85-
ModSetsConfig.disabledMods.addAll(mods)
85+
ModSets.config.disabledMods.addAll(mods)
8686
}
8787
},
8888
)
@@ -99,7 +99,7 @@ data class BooleanRule(val mod: String) : OptionRule<Boolean> {
9999
.apply {
100100
(
101101
rule.description
102-
?: Rules.modSets[mod]?.description
102+
?: ModSets.rules.modSets[mod]?.description
103103
)?.let { description(OptionDescription.of(it)) }
104104
}
105105
.instant(true)
@@ -124,7 +124,7 @@ data class CyclingRule(val mods: List<String>) : OptionRule<String> {
124124
CyclingListControllerBuilder.create(it)
125125
.values(mods)
126126
.valueFormatter { mod ->
127-
val modSet = Rules.modSets.getOrThrow(mod)
127+
val modSet = ModSets.rules.modSets.getOrThrow(mod)
128128
modSet.text.copy()
129129
.withStyle(
130130
Style.EMPTY.withHoverEvent(
@@ -144,31 +144,33 @@ data class CyclingRule(val mods: List<String>) : OptionRule<String> {
144144
Binding.generic(
145145
firstMod,
146146
{
147-
val modSets = Rules.modSets
147+
val modSets = ModSets.rules.modSets
148148
val enabledModSet = mods.asSequence()
149149
.filter { modSet ->
150150
val mods = modSets.getOrThrow(modSet).mods
151-
mods.isNotEmpty() && mods.none { it in ModSetsConfig.disabledMods }
151+
mods.isNotEmpty() && mods.none { it in ModSets.config.disabledMods }
152152
}
153153
.toList()
154154
if (enabledModSet.size > 1) {
155155
ModSets.logger.warn("More than one mod is enabled in cycling list: " + enabledModSet.joinToString() + ". Will take the first and disable the others")
156-
ModSetsConfig.disabledMods.addAll(
156+
ModSets.config.disabledMods.addAll(
157157
enabledModSet.drop(1).flatMap { modSets.getOrThrow(it).mods },
158158
)
159-
ModSetsConfig.disabledMods.removeAll(modSets.getOrThrow(enabledModSet.first()).mods.toSet())
159+
ModSets.config.disabledMods.removeAll(modSets.getOrThrow(enabledModSet.first()).mods.toSet())
160160
return@generic enabledModSet.first()
161161
}
162162
val currentSelected =
163-
enabledModSet.singleOrNull() ?: mods.firstOrNull { modSets.getOrThrow(it).mods.isEmpty() }
163+
enabledModSet.singleOrNull { modSet ->
164+
modSets.getOrThrow(modSet).mods.none { it in ModSets.config.disabledMods }
165+
} ?: mods.firstOrNull { modSets.getOrThrow(it).mods.isEmpty() }
164166
?: firstMod
165167

166-
ModSetsConfig.disabledMods.removeAll(modSets.getOrThrow(currentSelected).mods.toSet())
168+
ModSets.config.disabledMods.removeAll(modSets.getOrThrow(currentSelected).mods.toSet())
167169
return@generic currentSelected
168170
},
169171
) { value: String ->
170-
ModSetsConfig.disabledMods.addAll(mods.flatMap { Rules.modSets.getOrThrow(it).mods })
171-
ModSetsConfig.disabledMods.removeAll(Rules.modSets.getOrThrow(value).mods.toSet())
172+
ModSets.config.disabledMods.addAll(mods.flatMap { ModSets.rules.modSets.getOrThrow(it).mods })
173+
ModSets.config.disabledMods.removeAll(ModSets.rules.modSets.getOrThrow(value).mods.toSet())
172174
},
173175
).build()
174176
}
@@ -187,7 +189,7 @@ data class ModsGroupRule(val mods: List<String>, val collapsed: Boolean = true)
187189
val group = OptionGroup.createBuilder().name(rule.text)
188190
rule.description?.let { group.description(OptionDescription.of(it)) }
189191
for (mod in mods) {
190-
val modSet = Rules.modSets.getOrThrow(mod)
192+
val modSet = ModSets.rules.modSets.getOrThrow(mod)
191193
val option = Option.createBuilder<Boolean>().name(modSet.text)
192194
modSet.description?.let { option.description(OptionDescription.of(it)) }
193195
group.option(

‎common/src/game/kotlin/settingdust/modsets/game/Rules.kt

+22-27
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ package settingdust.modsets.game
22

33
import dev.isxander.yacl3.api.*
44
import dev.isxander.yacl3.api.controller.StringControllerBuilder
5-
import kotlinx.coroutines.flow.MutableSharedFlow
6-
import kotlinx.coroutines.flow.asSharedFlow
5+
import kotlinx.coroutines.delay
6+
import kotlinx.coroutines.flow.*
77
import kotlinx.coroutines.runBlocking
88
import kotlinx.serialization.ExperimentalSerializationApi
99
import kotlinx.serialization.json.Json
@@ -16,22 +16,19 @@ import net.minecraft.network.chat.Component
1616
import settingdust.kinecraft.serialization.ComponentSerializer
1717
import settingdust.kinecraft.serialization.GsonElementSerializer
1818
import settingdust.modsets.ModSets
19-
import settingdust.modsets.ModSetsConfig
2019
import settingdust.modsets.PlatformHelper
2120
import settingdust.modsets.config
2221
import kotlin.io.path.*
2322

2423
@OptIn(ExperimentalSerializationApi::class)
2524
@Deprecated("Use ModSets.rules instead", ReplaceWith("ModSets.rules"))
26-
object Rules : MutableMap<String, RuleSet> by mutableMapOf() {
25+
object Rules : MutableMap<String, RuleSet> by hashMapOf() {
2726
private val configDir = PlatformHelper.configDir / "modsets"
2827

29-
val modSets = mutableMapOf<String, ModSet>()
30-
private val _ModSetsRegisterCallback =
31-
MutableSharedFlow<Unit>()
32-
val ModSetsRegisterCallback = _ModSetsRegisterCallback.asSharedFlow()
28+
val modSets = hashMapOf<String, ModSet>()
29+
val ModSetsRegisterCallback = WaitedSharedFlow<Unit>()
3330

34-
private val definedModSets = mutableMapOf<String, ModSet>()
31+
private val definedModSets = hashMapOf<String, ModSet>()
3532
private val modSetsPath = configDir / "modsets.json"
3633

3734
private val rulesDir = configDir / "rules"
@@ -51,29 +48,29 @@ object Rules : MutableMap<String, RuleSet> by mutableMapOf() {
5148

5249
private val config: YetAnotherConfigLib
5350
get() {
54-
runBlocking { load() }
51+
load()
5552
val builder = YetAnotherConfigLib.createBuilder().title(Component.translatable("modsets.name"))
56-
if (ModSetsConfig.common.displayModSetsScreen && modSets.isNotEmpty()) {
53+
if (ModSets.config.common.displayModSetsScreen && modSets.isNotEmpty()) {
5754
builder.category(
5855
ConfigCategory.createBuilder().apply {
5956
name(Component.translatable("modsets.name"))
6057
tooltip(Component.translatable("modsets.description"))
6158
groups(
62-
modSets.map { modSet ->
59+
modSets.map { (name, modSet) ->
6360
ListOption.createBuilder<String>()
6461
.apply {
65-
name(modSet.value.text)
66-
modSet.value.description?.let { description(OptionDescription.of(it)) }
62+
name(modSet.text)
63+
modSet.description?.let { description(OptionDescription.of(it)) }
6764
initial("")
6865
collapsed(true)
6966
controller { StringControllerBuilder.create(it) }
7067
binding(
71-
Binding.generic(modSet.value.mods.toMutableList(), {
72-
modSet.value.mods.toMutableList()
68+
Binding.generic(modSet.mods.toMutableList(), {
69+
modSet.mods.toMutableList()
7370
}) {
74-
modSet.value.mods.clear()
75-
modSet.value.mods.addAll(it)
76-
definedModSets[modSet.key] = modSet.value
71+
modSet.mods.clear()
72+
modSet.mods.addAll(it)
73+
definedModSets[name] = modSet
7774
},
7875
)
7976
}
@@ -101,11 +98,9 @@ object Rules : MutableMap<String, RuleSet> by mutableMapOf() {
10198
for (option in options) {
10299
option.addListener { _, _ ->
103100
var needSave = false
104-
for (currentOption in options.filter { it != option }) {
105-
if (currentOption.changed()) {
106-
needSave = true
107-
(currentOption as Option<Any>).requestSet(currentOption.binding().value)
108-
}
101+
options.filter { it != option && it.changed() }.forEach {
102+
needSave = true
103+
(it as Option<Any>).requestSet(it.binding().value)
109104
}
110105
if (option.changed()) {
111106
(option as Option<Any>).requestSet(option.binding().value)
@@ -126,10 +121,10 @@ object Rules : MutableMap<String, RuleSet> by mutableMapOf() {
126121
}
127122

128123
init {
129-
runBlocking { load() }
124+
load()
130125
}
131126

132-
private suspend fun load() {
127+
private fun load() {
133128
ModSets.config.load()
134129
try {
135130
configDir.createDirectories()
@@ -145,7 +140,7 @@ object Rules : MutableMap<String, RuleSet> by mutableMapOf() {
145140
definedModSets.putAll(json.decodeFromStream(it))
146141
}
147142
modSets.putAll(definedModSets)
148-
_ModSetsRegisterCallback.emit(Unit)
143+
runBlocking { ModSetsRegisterCallback.emit(Unit)}
149144

150145
clear()
151146
rulesDir.listDirectoryEntries("*.json").forEach {

‎common/src/game/kotlin/settingdust/modsets/game/WaitedSharedFlow.kt

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package settingdust.modsets.game
2+
3+
import kotlinx.coroutines.CoroutineScope
4+
import kotlinx.coroutines.Dispatchers
5+
import kotlinx.coroutines.flow.MutableSharedFlow
6+
import kotlinx.coroutines.flow.launchIn
7+
import kotlinx.coroutines.flow.onEach
8+
import kotlinx.coroutines.withContext
9+
import kotlin.coroutines.CoroutineContext
10+
11+
class WaitedSharedFlow<T>(private val context: CoroutineContext = Dispatchers.Default) {
12+
private val scope = CoroutineScope(context)
13+
14+
private val _events = MutableSharedFlow<T>()
15+
16+
suspend fun emit(event: T) = withContext(context) {
17+
_events.emit(event)
18+
}
19+
20+
fun subscribe(block: (event: T) -> Unit) = _events
21+
.onEach { block(it) }
22+
.launchIn(scope)
23+
}

‎fabric/gradle.properties

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
org.gradle.jvmargs=-Xmx2G

‎fabric/src/main/kotlin/settingdust/modsets/fabric/Entrypoint.kt

+6-3
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,20 @@ package settingdust.modsets.fabric
33
import kotlinx.coroutines.DelicateCoroutinesApi
44
import kotlinx.coroutines.Dispatchers
55
import kotlinx.coroutines.GlobalScope
6+
import kotlinx.coroutines.flow.collect
7+
import kotlinx.coroutines.flow.launchIn
8+
import kotlinx.coroutines.flow.onEach
69
import kotlinx.coroutines.launch
710
import net.fabricmc.api.ModInitializer
811
import net.fabricmc.loader.api.FabricLoader
912
import net.fabricmc.loader.api.metadata.ModOrigin
1013
import net.fabricmc.loader.impl.FabricLoaderImpl
1114
import net.minecraft.client.resources.language.I18n
1215
import net.minecraft.network.chat.Component
13-
import settingdust.modsets.game.ModSet
1416
import settingdust.modsets.ModSets
15-
import settingdust.modsets.game.Rules.ModSetsRegisterCallback
1617
import settingdust.modsets.config
18+
import settingdust.modsets.game.ModSet
19+
import settingdust.modsets.game.Rules.ModSetsRegisterCallback
1720
import settingdust.modsets.game.rules
1821
import kotlin.io.path.div
1922

@@ -25,7 +28,7 @@ object Entrypoint : ModInitializer {
2528
val modSets = ModSets.rules.modSets
2629

2730
GlobalScope.launch(Dispatchers.IO) {
28-
ModSetsRegisterCallback.collect {
31+
ModSetsRegisterCallback.subscribe {
2932
for ((key, value) in FilteredDirectoryModCandidateFinder.directoryModSets
3033
.mapValues {
3134
ModSet(

‎fabric/src/main/resources/fabric.mod.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"modmenu": [
2626
{
2727
"adapter": "kotlin",
28-
"value": "${group}.ModSetsModMenu"
28+
"value": "${group}.game.ModSetsModMenu"
2929
}
3030
]
3131
},

‎forge/src/core/kotlin/settingdust/modsets/forge/Entrypoint.kt

+8-6
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,21 @@ package settingdust.modsets.forge
22

33
import kotlinx.coroutines.Dispatchers
44
import kotlinx.coroutines.GlobalScope
5+
import kotlinx.coroutines.flow.collect
6+
import kotlinx.coroutines.flow.launchIn
7+
import kotlinx.coroutines.flow.onEach
58
import kotlinx.coroutines.launch
69
import net.minecraft.network.chat.Component
710
import net.minecraftforge.client.ConfigScreenHandler.ConfigScreenFactory
811
import net.minecraftforge.fml.ModList
912
import net.minecraftforge.fml.common.Mod
1013
import net.minecraftforge.fml.loading.FMLPaths
11-
import net.minecraftforge.fml.loading.moddiscovery.*
12-
import settingdust.modsets.*
14+
import net.minecraftforge.fml.loading.moddiscovery.ModsFolderLocator
15+
import settingdust.modsets.ModSets
16+
import settingdust.modsets.config
1317
import settingdust.modsets.forge.service.ModSetsModLocator
1418
import settingdust.modsets.game.ModSet
19+
import settingdust.modsets.game.Rules
1520
import settingdust.modsets.game.rules
1621
import thedarkcolour.kotlinforforge.forge.LOADING_CONTEXT
1722
import kotlin.io.path.div
@@ -20,9 +25,6 @@ import kotlin.io.path.div
2025
class Entrypoint {
2126
init {
2227
// Take from https://github.com/isXander/YetAnotherConfigLib/blob/1.20.x/dev/test-forge/src/main/java/dev/isxander/yacl/test/forge/ForgeTest.java
23-
val gameClassLoader = javaClass.classLoader
24-
// val rulesClass = gameClassLoader.loadClass("settingdust.modsets.Rules")
25-
// val rules = rulesClass.getDeclaredField("INSTANCE")[null] as Rules
2628
LOADING_CONTEXT.registerExtensionPoint(ConfigScreenFactory::class.java) {
2729
ConfigScreenFactory { _, parent ->
2830
ModSets.rules.createScreen(parent)
@@ -34,7 +36,7 @@ class Entrypoint {
3436
val modSets = ModSets.rules.modSets
3537

3638
GlobalScope.launch(Dispatchers.IO) {
37-
ModSets.rules.ModSetsRegisterCallback.collect {
39+
ModSets.rules.ModSetsRegisterCallback.subscribe {
3840
for ((key, value) in ModSetsModLocator.directoryModSet.mapValues {
3941
ModSet(
4042
Component.literal(it.key),

‎gradle.properties

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ kotlin.stdlib.default.dependency=false
44
org.gradle.jvmargs=-Xmx4G
55
#org.gradle.parallel=true
66
# Mod Properties
7-
mod_version=1.2.0
7+
mod_version=1.2.1
88
maven_group=settingdust.modsets
99
archives_name=mod_sets
1010
mod_name=Mod Sets

0 commit comments

Comments
 (0)