Skip to content

Simplify item ID resolution to drop module-prefixed lookup #328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Travja merged 4 commits into from
Sep 12, 2026
Merged

Simplify item ID resolution to drop module-prefixed lookup #328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d5947d4
Simplify item ID resolution to drop module-prefixed lookup
claude Jul 23, 2026
be7f123
Keep isCustomItemOfId backward compatible with legacy namespaced IDs
claude Jul 24, 2026
ae47e7e
Reject ambiguous bare item ids instead of picking one silently
Travja Sep 12, 2026
80fbfcf
Make DivinityItemType.getID() module-prefixed when the id is ambiguous
Travja Sep 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 37 additions & 10 deletions src/main/java/studio/magemonkey/divinity/utils/DivinityProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@
import studio.magemonkey.divinity.modules.list.itemgenerator.ItemGeneratorManager;
import studio.magemonkey.divinity.stats.items.ItemStats;

import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

public class DivinityProvider implements ICodexItemProvider<DivinityProvider.DivinityItemType> {
public static final String NAMESPACE = "DIVINITY";
Expand Down Expand Up @@ -82,12 +85,18 @@ public DivinityItemType getItem(String id) {
IModule<?> module = Divinity.getInstance().getModuleManager().getModule(split[0]);
if (!(module instanceof QModuleDrop)) return null;
moduleItem = ((QModuleDrop<?>) module).getItemById(split[1]);
} else { // Look in all modules
for (IModule<?> module : Divinity.getInstance().getModuleManager().getModules()) {
if (!(module instanceof QModuleDrop)) continue;
} else { // Look in all modules; require the id to be unambiguous
Map<IModule<?>, ModuleItem> matches = findModuleItemsById(id);

if (matches.size() > 1) {
Codex.error("Ambiguous Divinity item id '" + id + "' found in multiple modules ("
+ matches.keySet().stream().map(IModule::getId).collect(Collectors.joining(", "))
+ "). Refer to it as '<module>:" + id + "' to disambiguate.");
return null;
}

moduleItem = ((QModuleDrop<? extends ModuleItem>) module).getItemById(id);
if (moduleItem != null) break;
if (!matches.isEmpty()) {
moduleItem = matches.values().iterator().next();
}
}

Expand All @@ -96,6 +105,20 @@ public DivinityItemType getItem(String id) {
return null;
}

/**
* Finds every module that has an item registered under the given plain id.
*/
private static Map<IModule<?>, ModuleItem> findModuleItemsById(String id) {
Map<IModule<?>, ModuleItem> matches = new LinkedHashMap<>();
for (IModule<?> module : Divinity.getInstance().getModuleManager().getModules()) {
if (!(module instanceof QModuleDrop)) continue;

ModuleItem candidate = ((QModuleDrop<? extends ModuleItem>) module).getItemById(id);
if (candidate != null) matches.put(module, candidate);
}
return matches;
}

@Override
@Nullable
public DivinityProvider.DivinityItemType getItem(ItemStack itemStack) {
Expand All @@ -119,12 +142,12 @@ public boolean isCustomItemOfId(ItemStack item, String id) {

String itemId = ItemStats.getId(item);
if (itemId == null) return false;
if (itemId.equals(id)) return true;

// Backward compatibility: older callers may still pass the legacy
// "module:id" namespaced form this method used to require.
String[] split = id.split(":", 2);
if (split.length < 2) {
return itemId.equals(id);
}

if (split.length < 2) return false;
QModuleDrop<?> module = ItemStats.getModule(item);
return module != null && module.getId().equalsIgnoreCase(split[0]) && itemId.equals(split[1]);
}
Expand Down Expand Up @@ -158,7 +181,11 @@ public String getNamespace() {

@Override
public String getID() {
return this.moduleItem.getModule().getId() + ":" + this.moduleItem.getId();
String id = this.moduleItem.getId();
if (findModuleItemsById(id).size() > 1) {
return this.moduleItem.getModule().getId() + ":" + id;
}
return id;
}

@Override
Expand Down
100 changes: 76 additions & 24 deletions src/test/java/studio/magemonkey/divinity/utils/DivinityProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
package studio.magemonkey.divinity.utils;

import org.bukkit.Material;
import org.bukkit.inventory.ItemStack;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.MockedStatic;
import org.bukkit.inventory.ItemStack;
import studio.magemonkey.codex.Codex;
import studio.magemonkey.codex.CodexEngine;
import studio.magemonkey.codex.items.CodexItemManager;
import studio.magemonkey.codex.modules.ModuleManager;
import studio.magemonkey.divinity.Divinity;
import studio.magemonkey.divinity.modules.api.QModuleDrop;
import studio.magemonkey.divinity.modules.list.arrows.ArrowManager;
import studio.magemonkey.divinity.modules.list.customitems.CustomItemsManager;
import studio.magemonkey.divinity.modules.list.itemgenerator.ItemGeneratorManager;
Expand Down Expand Up @@ -62,13 +61,14 @@ void setUp() {

itemGenModule = spy(new ItemGeneratorManager(divinity));
when(moduleManager.getModule("item_generator")).thenReturn(itemGenModule);
when(moduleManager.getModules()).thenReturn(List.of(arrowModule, itemGenModule));
doReturn("item_generator").when(itemGenModule).getId();

customItemsModule = spy(new CustomItemsManager(divinity));
when(moduleManager.getModule("custom_items")).thenReturn(customItemsModule);
doReturn("custom_items").when(customItemsModule).getId();

when(moduleManager.getModules()).thenReturn(List.of(arrowModule, itemGenModule, customItemsModule));

//noinspection unchecked
when(divinity.getModuleManager()).thenReturn(moduleManager);
}
Expand All @@ -83,8 +83,6 @@ void afterEach() {
void getItem_usesLevel() {
ItemGeneratorManager.GeneratorItem generatorItem = mock(ItemGeneratorManager.GeneratorItem.class);
doReturn(generatorItem).when(itemGenModule).getItemById("foobar");
when(generatorItem.getId()).thenReturn("foobar");
doReturn((QModuleDrop<?>) itemGenModule).when(generatorItem).getModule();

DivinityProvider.DivinityItemType item = provider.getItem("DIVINITY_item_generator:foobar~level:5");

Expand All @@ -100,8 +98,6 @@ void getItem_usesLevel() {
void getItem_usesMaterial() {
ItemGeneratorManager.GeneratorItem generatorItem = mock(ItemGeneratorManager.GeneratorItem.class);
doReturn(generatorItem).when(itemGenModule).getItemById("foobar");
when(generatorItem.getId()).thenReturn("foobar");
doReturn((QModuleDrop<?>) itemGenModule).when(generatorItem).getModule();

DivinityProvider.DivinityItemType item =
provider.getItem("DIVINITY_item_generator:foobar~material:VANILLA_DIAMOND");
Expand All @@ -119,8 +115,6 @@ void getItem_usesMaterial() {
void getItem_noModule_returnsItem() {
ItemGeneratorManager.GeneratorItem generatorItem = mock(ItemGeneratorManager.GeneratorItem.class);
doReturn(generatorItem).when(itemGenModule).getItemById("foobar");
when(generatorItem.getId()).thenReturn("foobar");
doReturn((QModuleDrop<?>) itemGenModule).when(generatorItem).getModule();

DivinityProvider.DivinityItemType item = provider.getItem("DIVINITY_foobar");

Expand All @@ -136,8 +130,6 @@ void getItem_noModule_returnsItem() {
void getItem_customItems_returnsItem() {
CustomItemsManager.CustomItem codexItem = mock(CustomItemsManager.CustomItem.class);
doReturn(codexItem).when(customItemsModule).getItemById("foobar");
when(codexItem.getId()).thenReturn("foobar");
doReturn((QModuleDrop<?>) customItemsModule).when(codexItem).getModule();

DivinityProvider.DivinityItemType item = provider.getItem("DIVINITY_custom_items:foobar");

Expand All @@ -149,19 +141,6 @@ void getItem_customItems_returnsItem() {
assertInstanceOf(DivinityProvider.DivinityItemType.class, item);
}

@Test
void getItem_namespacedIdIncludesModule() {
CustomItemsManager.CustomItem codexItem = mock(CustomItemsManager.CustomItem.class);
doReturn(codexItem).when(customItemsModule).getItemById("foobar");
when(codexItem.getId()).thenReturn("foobar");
doReturn((QModuleDrop<?>) customItemsModule).when(codexItem).getModule();

DivinityProvider.DivinityItemType item = provider.getItem("DIVINITY_custom_items:foobar");

assertNotNull(item);
assertEquals("DIVINITY_custom_items:foobar", item.getNamespacedID());
}

@Test
void getItem_itemStackUsesStoredModule() {
ItemStack itemStack = mock(ItemStack.class);
Expand All @@ -184,4 +163,77 @@ void getItem_itemStackUsesStoredModule() {
verify(itemGenModule, never()).getItemById("foobar");
}
}

@Test
void getItem_itemStackWithoutStoredModule_fallsBackToBareIdLookup() {
ItemStack itemStack = mock(ItemStack.class);

ItemGeneratorManager.GeneratorItem generatorItem = mock(ItemGeneratorManager.GeneratorItem.class);
doReturn(generatorItem).when(itemGenModule).getItemById("foobar");

try (MockedStatic<ItemStats> itemStats = mockStatic(ItemStats.class)) {
itemStats.when(() -> ItemStats.getId(itemStack)).thenReturn("foobar");
itemStats.when(() -> ItemStats.getModule(itemStack)).thenReturn(null);

DivinityProvider.DivinityItemType item = provider.getItem(itemStack);

assertNotNull(item);
assertEquals(generatorItem, item.getModuleItem());
}
}

@Test
void getItem_ambiguousBareId_returnsNull() {
ItemGeneratorManager.GeneratorItem generatorItem = mock(ItemGeneratorManager.GeneratorItem.class);
doReturn(generatorItem).when(itemGenModule).getItemById("foobar");

CustomItemsManager.CustomItem codexItem = mock(CustomItemsManager.CustomItem.class);
doReturn(codexItem).when(customItemsModule).getItemById("foobar");

DivinityProvider.DivinityItemType item = provider.getItem("DIVINITY_foobar");

assertNull(item);
}

@Test
void getItem_ambiguousId_stillResolvableWithModulePrefix() {
ItemGeneratorManager.GeneratorItem generatorItem = mock(ItemGeneratorManager.GeneratorItem.class);
doReturn(generatorItem).when(itemGenModule).getItemById("foobar");

CustomItemsManager.CustomItem codexItem = mock(CustomItemsManager.CustomItem.class);
doReturn(codexItem).when(customItemsModule).getItemById("foobar");

DivinityProvider.DivinityItemType item = provider.getItem("DIVINITY_custom_items:foobar");

assertNotNull(item);
assertEquals(codexItem, item.getModuleItem());
}

@Test
void getID_uniqueId_returnsBareId() {
CustomItemsManager.CustomItem codexItem = mock(CustomItemsManager.CustomItem.class);
doReturn(codexItem).when(customItemsModule).getItemById("foobar");
when(codexItem.getId()).thenReturn("foobar");

DivinityProvider.DivinityItemType item = provider.getItem("DIVINITY_custom_items:foobar");

assertNotNull(item);
assertEquals("foobar", item.getID());
}

@Test
void getID_ambiguousId_returnsModulePrefixedId() {
ItemGeneratorManager.GeneratorItem generatorItem = mock(ItemGeneratorManager.GeneratorItem.class);
doReturn(generatorItem).when(itemGenModule).getItemById("foobar");

CustomItemsManager.CustomItem codexItem = mock(CustomItemsManager.CustomItem.class);
doReturn(codexItem).when(customItemsModule).getItemById("foobar");
when(codexItem.getId()).thenReturn("foobar");
doReturn(customItemsModule).when(codexItem).getModule();

DivinityProvider.DivinityItemType item = provider.getItem("DIVINITY_custom_items:foobar");

assertNotNull(item);
assertEquals("custom_items:foobar", item.getID());
}
}
Loading