Sponge: Broken CatalogTypes

[ST-DDT]

Based on rejected PR #387.

Currently existing issues:

• Existing CatalogTypes not being accessible via GameRegistry
• CatalogType's getId() method not being implemented or throwing errors
• CatalogType's getName() method not being implemented or throwing errors
• CatalogType's getTranslation() method not being implemented or throwing errors or returning the wrong Translations (AFAICT)
• CatalogType's toString() method only using the default implementation or throwing errors
• GameRegistry#getAll(Class) returning duplicate instances
• GameRegistry's getType(type, id) returning absent or errors for valid ids
• GameRegistry's getType(type, id) throwing an Exception instead of Optional.empty() if a key is missing/invalid (but != null)
• Classes mentioned in @CatalogedBy containing public static fields with null values

Test-Plugin

• Plugin.jar
• Plugin sources

Wait for GameStartedServerEvent and you will get an overview about what is and isn't working.

The plugin searches for the following error types:

• No registry module for CatalogType class (GameRegistry#getAll() fails)
• CatalogType#getId() not implemented, errors or returns null
• CatalogType#getName() not implemented, errors or returns null
• CatalogType#getTranslation() not implemented, errors or returns null
• CatalogType#getTranslation() returns a Translation that errors if used
• CatalogType#getTranslation() returns a Translation that has no translation on the server
• CatalogType#toString() errors or returns null
• GameRegistry#getAll(Class) returns duplicate instances
• GameRegistry#getType(Class, String) errors
• GameRegistry#getType(Class, String) returns no instance for an id that was obtained from an instance from GameRegistry#getAll(Class)
• GameRegistry#getType(Class, String) returns a different instance for an id that was obtained from an instance from GameRegistry#getAll(Class)
• GameRegistry#getType(Class, String) errors when requesting an id that does not exist
• Classes referenced by CatalogTypes via @CatalogedBy containing null values in static fields

(Checks are limited to CatalogTypes that are listed in CatalogTypes)

Tested with spongeforge-1.8.9-1890-4.1.0-BETA-1347
Summary
Errors

See PR #387 for potential fixes and more hints.

[gabizou]

Most all of this should be fixed. Care to verify any additional issues?

[ST-DDT]

Yes. I will try to do this this evening or this WE.

[ST-DDT]

@gabizou: Tested with spongeforge-1.11.2-2227-6.0.0-BETA-2242.jar:

Most of the errors have been fixed but there are still ~25 issues left
(~1900 single failures total , ~1700 being missing translations (broken translation keys) for Statistics).

CatalogTest-Short.log

Test-Plugin.java

EDIT: 2244 has the same errors.

[ST-DDT]

@gabizou Added some more tests and found some registries that did not contain any entries.

TestCases:

• Is not a catalog type but listed as one: clazz
• Failed entirely on: clazz
• Failed id on: clazz
• Failed name on: clazz
• Failed translate on: clazz
• Failed server translation on class: clazz
• Missing server translation on key: 'CTType: id' = 'translationId'
• Failed toString on: clazz
• Failed duplicates on: clazz
• Failed registry.getType entirely on: clazz
• Failed registry.getType (single id) on: type with id id
• Failed registry.getType result for: type
• Failed registry.getType absent result on: clazz
• Found null result in: field
• Found dummy result in: field
• Found broken result in: field
• getAllOF did not return any results: clazz
• getAllOF did return a mutable result (Copy): clazz
• getAllOF did return a mutable result (Not a copy): clazz

EDIT 2017-04-02

• CatalogType's methods not working (no param ones only)

[ST-DDT]

@gabizou I ran some more tests on other methods on CatalogTypes and found the following issues:

• BlockType#getTickRandomly() - StackOverflow (Should be shadow$getTickRandomly())
• ArmorType#getRepairItemType() - AbstractMethod
• PotionEffectType#isInstant() - StackOverflow (Should be shadow$isInstant())
• Statistic#getType() - NoSuchElementException (Should be substring(0, index) (Missing start from zero))
• MixinEnumLogAxis/Cycleable#cycleNext() - ArrayIndexOutOfBoundsException (Should be (ordinal + 1) % length (Missing braces))

Tested version: spongeforge-1.11.2-2227-6.0.0-BETA-2260

EDIT:

This is a junit test that should test it as well:

````java
@Test
public void testCatalogMethodImpl() {
Map failedMethods =
new TreeMap<>(Comparator.comparing(m -> m.getClass().getName()).thenComparing(Method::getName));
for (Method method : getTestableApiMethods(getApplicableApiCatalogTypeInterfaces(this.catalogType))) {
try {
checkNotNull(method.invoke(this.catalogType), "Value from " + method.getDeclaringClass() + '#' + method.getName());
} catch (Throwable t) {
if (t instanceof InvocationTargetException && t.getCause() != null) {
t = t.getCause();
}
failedMethods.put(method, t);
}
}
if (failedMethods.isEmpty()) {
AssertionFailedError error =
new AssertionFailedError("There are failing API methods [" + failedMethods.keySet() + "] on " + this.catalogClass.getName() + ": "
+ this.catalogId);
failedMethods.values().forEach(error::addSuppressed);
throw error;
}
}

static Stream> getApplicableApiInterfaces(Object instance) {
Collection> interfaces = new LinkedHashSet<>();
Class current = instance.getClass();
while (current != null) {
interfaces.addAll(Arrays.asList(current.getInterfaces()));
current = current.getSuperclass();
}
return interfaces.stream()
.filter(clazz -> clazz.getName().startsWith("org.spongepowered.api."));
}

static Stream> getApplicableApiCatalogTypeInterfaces(Object instance) {
return getApplicableApiInterfaces(instance)
.filter(CatalogType.class::isAssignableFrom)
.map(clazz -> clazz.asSubclass(CatalogType.class));
}

static Set getTestableApiMethods(Stream> clazzes) {
return clazzes.map(Class::getMethods)
.flatMap(Arrays::stream)
.filter(m -> m.getParameters().length == 0)
.collect(Collectors.toSet());
}
````

Unfortunately the mcp mapping is offline thus i cannot test the test.

[parlough]

@ST-DDT What is the current state of this issue? What still needs to be addressed?

[ImMorpheus]

@Meronat I'm on it. This requires a rework of Statistics/StatisticTypes (and probably a few other things).

[parlough]

@ImMorpheus Keep in mind that statistics need some changes in 1.13, so best to keep in contact with cyber for any changes you want to make or bring them up on his PR maybe. In the PR he is making necessary changes while also cleaning up the API a bit already, so maybe the changes can be bundled together. You can find his WIP PR here - https://github.com/SpongePowered/SpongeAPI/pull/1742

[gabizou]

I know this is a long standing issue, but if any of these can be re-tested, I've already verified that the issues covered from this comment are fixed:

@gabizou I ran some more tests on other methods on CatalogTypes and found the following issues:

• BlockType#getTickRandomly() - StackOverflow (Should be shadow$getTickRandomly())
• ArmorType#getRepairItemType() - AbstractMethod
• PotionEffectType#isInstant() - StackOverflow (Should be shadow$isInstant())
• Statistic#getType() - NoSuchElementException (Should be substring(0, index) (Missing start from zero))
• MixinEnumLogAxis/Cycleable#cycleNext() - ArrayIndexOutOfBoundsException (Should be (ordinal + 1) % length (Missing braces))

Tested version: spongeforge-1.11.2-2227-6.0.0-BETA-2260

As for the remainder of issues, I believe a majority of them are pending for API 8 to be resolved.