Skip to content
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.

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

Electrical Age integration crash #110

Open
mpevner opened this issue Mar 17, 2017 · 9 comments
Open

Electrical Age integration crash #110

mpevner opened this issue Mar 17, 2017 · 9 comments

Comments

@mpevner
Copy link

mpevner commented Mar 17, 2017

With the latest versions of Electrical Age, it is possible to induce a crash viewing recipes. I have traced this down to a spelling issue, being a s->z change.
The needed fix is to simply to change Eln.instance.magnetiserRecipes to Eln.instance.magnetizerRecipes in the RecipeHandlerMagnetizer and to use a newer version of ELN as the dependancy. Or so it seems.

@jrddunbr
Copy link

jrddunbr commented May 5, 2018

This problem is also expanded to some fields being private instead of public now. I've been able to make a build of Electrical Age that only crashes on that spelling correction (so I think)

Electrical-Age/ElectricalAge#849

I'm now working on making an edit or two and perhaps a push request to this repository.

Edit: Can't seem to get an API jar for a newer version to compile :(

@jrddunbr
Copy link

jrddunbr commented May 5, 2018

So I compiled ElectricalAge with some fields public and changed the spelling back to an s for that one variable, and it worked again. If we can't get a newer dev jar to change that one error, it may be worth pushing a new PR to Electrical Age. Regardless, in the newest version available on Forge's website, the variables required are private, and thus a PR is required either way on the Electrical Age repository.

@mpevner
Copy link
Author

mpevner commented May 5, 2018

If you want to comment further, I've already opened up an issue on ELN's side -- Electrical-Age/ElectricalAge#693. As for renaming to z on their side, I am hesitant as to any second order crashes that might cause (other integrations expecting an s).

@jrddunbr
Copy link

jrddunbr commented May 5, 2018

Indeed. Probably best to just leave Integrations alone and fix it on the Eln side, change it back to an s.
Perhaps I should do a PR with the code I have working...

@mpevner
Copy link
Author

mpevner commented May 5, 2018

Ah, to clarify, leaving the ELN side alone in case there are other integration mods out there. If Tonius is not responsive here, mayhaps on Curesforge?

@jrddunbr
Copy link

jrddunbr commented May 5, 2018

One way or the other, a change is needed to make some fields public on their codebase. If I could get a API jar compiled, then I would make a PR here and there.

@mpevner
Copy link
Author

mpevner commented May 5, 2018

Of relevance then -- Electrical-Age/ElectricalAge#652 ; it's not live Yet, it seems the ELN folks have gone on hiatus of sorts. For the future however...

@jrddunbr
Copy link

jrddunbr commented May 5, 2018

1.10 and 1.12 rewrite in progress. They don't want to focus on new features for 1.7.10.

Honestly, for the best future for the mod. Many people (incl myself) would love a 1.12 version.

@bloxgate
Copy link

bloxgate commented May 9, 2018

Hi. Author of that API PR here. I believe the only thing holding up its merging is that it needs to be rebased now that Baughn is actually able to review things again.

I should be able to take care of that in a couple days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants