Mekanism 8 integration issue #1061

Closed
accessdenied0 opened this Issue Mar 21, 2015 · 16 comments

Comments

Projects
None yet
6 participants
@accessdenied0

Dear AppliedEnergistics team,

I'm here to inform you for an issue with AE2 (rv2.beta build 14) and Mekanism. The issue is in combination with NEI, because the Client starts up and the world loads normally, but If I try to get recipe-information from any Item or Block in the list it crashes. I posted already in Mekanism the issue aidancbrady/Mekanism#2280 (comment).
But it seems to be an integration issue with the newest Mekanism API. (http://pastebin.com/xZimU9XS)

I hope I did it right,

access_denied

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch Mar 21, 2015

Member

I am waiting for a reponse on aidancbrady/Mekanism#2249 or I need @FireBall1725 to update the API jar

Member

thatsIch commented Mar 21, 2015

I am waiting for a reponse on aidancbrady/Mekanism#2249 or I need @FireBall1725 to update the API jar

@yueh

This comment has been minimized.

Show comment
Hide comment
@yueh

yueh Mar 21, 2015

Member

It might be an idea to add every used class to the class existance test to fail and disable the integration itself instead of letting the exception propagate into other parts. Maybe also check every other integration, if they not miss to test some classes.

It looks like there is no maven repository for mekanism (or just the API), so we probably have to host it on our repistory. I would appreciate it being listed under the right group:artificat:version instead of the currently incremented number of uploads and making it impossible to recognize the actually used version.

Member

yueh commented Mar 21, 2015

It might be an idea to add every used class to the class existance test to fail and disable the integration itself instead of letting the exception propagate into other parts. Maybe also check every other integration, if they not miss to test some classes.

It looks like there is no maven repository for mekanism (or just the API), so we probably have to host it on our repistory. I would appreciate it being listed under the right group:artificat:version instead of the currently incremented number of uploads and making it impossible to recognize the actually used version.

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch Mar 21, 2015

Member

There are actually a bunch missing, but I am not even sure, if we should check this, since it will fail anyways if we call it. Either way, we will get a ClassNotFoundException

I agree, alternative, we get access to the artifactory. My account of the build server does not work in that part.

Member

thatsIch commented Mar 21, 2015

There are actually a bunch missing, but I am not even sure, if we should check this, since it will fail anyways if we call it. Either way, we will get a ClassNotFoundException

I agree, alternative, we get access to the artifactory. My account of the build server does not work in that part.

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch Mar 21, 2015

Member

The API is located in the MDK, I think there was some gradle plugin which can simulate a maven dependency by using public links. Maybe use that?

Member

thatsIch commented Mar 21, 2015

The API is located in the MDK, I think there was some gradle plugin which can simulate a maven dependency by using public links. Maybe use that?

@yueh

This comment has been minimized.

Show comment
Hide comment
@yueh

yueh Mar 21, 2015

Member

We can just use Ivy instead of Maven for the dependency resolution and point to a random HTTP URL. Maybe directly to a build on the mekanism CI.

This might work. (Cannot test currently)

ivy {
    name 'Mekanism'
    artifactPattern "http://ci.indiewikis.com:8080/job/Mekanism/builds/[revision]/archive/output/[module]-1.7.10-8.0.0.[revision].[ext]"
}

And then add module MDK with revison 188 as dependency (the currently recommended build for 8.0). But the MDK is released as .zip and not .jar. Not sure if Ivy likes that.
And in general it is a bit hacky compared to a normal maven reposity. (As we have to refer to a specific build number with some hardcoded pattern containing the actual version and not a general version)

Member

yueh commented Mar 21, 2015

We can just use Ivy instead of Maven for the dependency resolution and point to a random HTTP URL. Maybe directly to a build on the mekanism CI.

This might work. (Cannot test currently)

ivy {
    name 'Mekanism'
    artifactPattern "http://ci.indiewikis.com:8080/job/Mekanism/builds/[revision]/archive/output/[module]-1.7.10-8.0.0.[revision].[ext]"
}

And then add module MDK with revison 188 as dependency (the currently recommended build for 8.0). But the MDK is released as .zip and not .jar. Not sure if Ivy likes that.
And in general it is a bit hacky compared to a normal maven reposity. (As we have to refer to a specific build number with some hardcoded pattern containing the actual version and not a general version)

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch Mar 21, 2015

Member

mh after some testing, I do not think this is good, even though it would be convenient.

Maybe we can have a new repo on here, for exactly that purpose (update the dependencies on the artfactory depending on other repositories), manually updating is a pain

Member

thatsIch commented Mar 21, 2015

mh after some testing, I do not think this is good, even though it would be convenient.

Maybe we can have a new repo on here, for exactly that purpose (update the dependencies on the artfactory depending on other repositories), manually updating is a pain

@yueh

This comment has been minimized.

Show comment
Hide comment
@yueh

yueh Mar 21, 2015

Member

I am not sure if artifactory can act as proxy to custom ivy repos (might be limited to the pro version)
So we might have to fall back to manually uploading them to our maven repo until every dependency is available throug a maven repo. There are even plugins for jenkins to act as maven repo.

But discussing it on IRC is probably better than spamming the mekanism issue with general things.

Member

yueh commented Mar 21, 2015

I am not sure if artifactory can act as proxy to custom ivy repos (might be limited to the pro version)
So we might have to fall back to manually uploading them to our maven repo until every dependency is available throug a maven repo. There are even plugins for jenkins to act as maven repo.

But discussing it on IRC is probably better than spamming the mekanism issue with general things.

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch Mar 22, 2015

Member

Just saying we could have something like a artifactory project, which has all repos in them we currently use, and then an api build task for each of these sub projects

Member

thatsIch commented Mar 22, 2015

Just saying we could have something like a artifactory project, which has all repos in them we currently use, and then an api build task for each of these sub projects

@halvors

This comment has been minimized.

Show comment
Hide comment
@halvors

halvors Mar 23, 2015

Any progress on this?

halvors commented Mar 23, 2015

Any progress on this?

@halvors

This comment has been minimized.

Show comment
Hide comment
@halvors

halvors Mar 23, 2015

@thatsIch You had Mekanism v7 integration right? How did you solve this before?

halvors commented Mar 23, 2015

@thatsIch You had Mekanism v7 integration right? How did you solve this before?

@mSt3in mSt3in referenced this issue in aidancbrady/Mekanism Mar 23, 2015

Closed

Breaking AE2 recipes. #2324

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch Mar 24, 2015

Member

the same as we do now too? But cant except, if somebody else breaks his own API, that it is fixed instantly. We have other things to do, too.

Member

thatsIch commented Mar 24, 2015

the same as we do now too? But cant except, if somebody else breaks his own API, that it is fixed instantly. We have other things to do, too.

@Morpheus1101

This comment has been minimized.

Show comment
Hide comment
@Morpheus1101

Morpheus1101 Mar 26, 2015

Note this requires a small edit in AE2: /src/main//java/appeng/integration/modules/Mekanism.java
from:
import mekanism.api.RecipeHelper;
to:
import mekanism.api.recipe.RecipeHelper;

(Thats apparently everything thats being reported about the issue with mekanism an how to solve it)

Note this requires a small edit in AE2: /src/main//java/appeng/integration/modules/Mekanism.java
from:
import mekanism.api.RecipeHelper;
to:
import mekanism.api.recipe.RecipeHelper;

(Thats apparently everything thats being reported about the issue with mekanism an how to solve it)

@thatsIch thatsIch closed this in 33c1535 Mar 26, 2015

thatsIch added a commit that referenced this issue Mar 26, 2015

Merge pull request #1086 from thatsIch/b-1061-mekanism
Fixes #1061 Recipes are properly registered with new Mekanism
@codewarrior0

This comment has been minimized.

Show comment
Hide comment
@codewarrior0

codewarrior0 Mar 27, 2015

I think the underlying problem here is that whenever AE2's integration with one mod breaks because a different version of that mod is installed, AE2 aborts registering ALL AE2 crafting recipes. AE2 should continue to register crafting recipes and skip the ones that depend on one mod's integration (in this case it cannot register the Crusher recipes). Disabling the one mod's integration in AE2's config is the current solution, but it would be much better if AE2 could disable the integration itself because the symptom of missing recipes is quite confusing and doesn't suggest making the fix to the config files, or even which mod's integration to disable.

In short, I think future-proofing the recipe registry against changes that happen in other mods is a good idea.

I think the underlying problem here is that whenever AE2's integration with one mod breaks because a different version of that mod is installed, AE2 aborts registering ALL AE2 crafting recipes. AE2 should continue to register crafting recipes and skip the ones that depend on one mod's integration (in this case it cannot register the Crusher recipes). Disabling the one mod's integration in AE2's config is the current solution, but it would be much better if AE2 could disable the integration itself because the symptom of missing recipes is quite confusing and doesn't suggest making the fix to the config files, or even which mod's integration to disable.

In short, I think future-proofing the recipe registry against changes that happen in other mods is a good idea.

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch Mar 27, 2015

Member

On the other hand, nobody would really notice, that a problem occured and thus nobody would care to report it, cause people are lazy.

It does not fail on non-existence, but on miss-behaviour. One is an exception, other is an error.

Member

thatsIch commented Mar 27, 2015

On the other hand, nobody would really notice, that a problem occured and thus nobody would care to report it, cause people are lazy.

It does not fail on non-existence, but on miss-behaviour. One is an exception, other is an error.

@codewarrior0

This comment has been minimized.

Show comment
Hide comment
@codewarrior0

codewarrior0 Mar 27, 2015

Starting up with a lot of AE2's own crafting recipes missing is kind of weird at best. I can think of two better responses to an error in the mod integration:

  • Print a message to the chat window about which mod's integration failed. Skip recipes that require that mod and continue loading the other recipes.
  • Crash hard and indicate a version mismatch.

Starting up with a lot of AE2's own crafting recipes missing is kind of weird at best. I can think of two better responses to an error in the mod integration:

  • Print a message to the chat window about which mod's integration failed. Skip recipes that require that mod and continue loading the other recipes.
  • Crash hard and indicate a version mismatch.
@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch Mar 27, 2015

Member

I wish we could integrate an auto-report functionality..

thats an idea.. mh

Member

thatsIch commented Mar 27, 2015

I wish we could integrate an auto-report functionality..

thats an idea.. mh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment