AppliedEnergistics / Applied-Energistics-2

A Minecraft Mod about Matter, Energy and using them to conquer the world..
https://appliedenergistics.github.io/
Other
1.44k stars 661 forks source link

[1.12] Is this mod still 'alive'? #2927

Closed electrofloat closed 7 years ago

electrofloat commented 7 years ago

The last commit was 4 months ago by Yueh.

Will this mod be updated to 1.12? Or it just simply depends on whether someone (who has commit rights) will magically have some time to get back to it?

And what about #2306 #2399 ?

mindforger commented 7 years ago

it is not dead, but i have no insight on what is mostly beein worked on by now aemodernport is mostly independend to the basic ae2 and the other issue got closed due to no discussion progress if you scroll up to yuehs last comment on this this mod is as stable yet as it could be imho, most issues are from external sources (AE2stuff, strange chunkloading, stacking storages with bad API behaviour) and some smaller forge problems and the global energy systems messup since 1.9

and to top up my comment as always: do not forget to add energy cells, an AE system can only work with an power input of "me system total CAPACITY"/tick and some operations can surge this capacity instantly without energy cells, then the system reboots and act strange

GuntherDW commented 7 years ago

I was giving it a go at updating to 1.11 when 1.12 wasn't out yet (IE, 2 weeks ago to last week), and have a bunch of stuff in a "dirty" branch that I need to clean up before committing. But I tried to contact yueh first in an attempt to find out if there was stuff that I would need to look out for or attempt to fix / ideas / goals for that version. Since then I haven't really worked on it because I wouldn't want to do any real work if it wasn't effectively going to get merged in the long run.

I was planning on resuming work even if yueh didn't respond last weekend, but a wedding party from one of my friends kind of screwed up my planning. Once a somewhat "stable" fork of 1.11 is done updating to 1.12 should be kind of easy I'd assume, but with the state of the codebase I wouldn't be too quick to declare that.

mgdgfbhevdyeh commented 7 years ago

it isn't dead,i think just yueh busy now.

GuntherDW commented 7 years ago

After not receiving any comment from yueh about whether or not he'd allow other people to do the update I decided to just bite the bullet and continue my work on the port. It's somewhat completed, with a few UI's displaying a tooltip where it shouldn't and rendering issues on cables+facades on a specific side.

https://i.imgur.com/dhlb0i5.png

The issue with rendering cables & select blocks that remain : http://i.imgur.com/SU0L5vR.png UI issue that I still have to fix : http://i.imgur.com/1tHZyza.png Strangely enough it doesn't happen in the interface, only in the interface terminal.

But P2P's, storage, buses (in/ex/storage), QNB's, assemblers, crafting cpu's, ... work atm.

I tried to stay as close to the AE codebase as possible, in order to not increase the chances of the work that I've put in going to waste eventually. Yes, it's 1.11.2, but seeing as there's no 1.12 version yet and the original "goal" was 1.11.2 I just decided to more or less complete it for 1.11.2 first.

Ommina commented 7 years ago

Last I checked (which was only a couple days ago), the 1.12 version of Forge was still a Beta release. I'd think any attempt at moving AE2 toward 1.12 would be premature at best and a distinct waste of time as worst.

But I do like that it stores air now. (And since this is the Internet, I have to point out that was humour. Really.)

electrofloat commented 7 years ago

@mgdgfbhevdyeh Algo abandoned the project long time ago. At around when the channel system was implemented. Users complained way too much about how complicated it is (which is not). (Much like Notch did with Minecraft itself. He got fed up with users complaining non-stop about everything.)

GuntherDW commented 7 years ago

I added an issue tracker on my fork of the repo to keep track of the port.

It'll be less off topic this way. https://github.com/GuntherDW/Applied-Energistics-2/issues/1

If you want to try out my port to find any bugs that I haven't catched yet you can do so at https://github.com/GuntherDW/Applied-Energistics-2/tree/rv5-1.11 . (No public prebuilt jars available just yet. It's a bit too early to put it into production packs.)

mgdgfbhevdyeh commented 7 years ago

@mydexterid i know those.

yueh commented 7 years ago

To make it short: No, not really alive.

This is mostly due to me lacking the required time for it. Of course this could potentially change at some point, but do not count on it.

On the other hand, if I actually have a bit of time to spend on it, then I am really lacking the motivation for it. Forge tries to pretty much everything to make the development harder and harder each time. Like I am no longer able to debug AE indev with any other mod present as normal .jar as the classloader completely screws up. Neither the normal or deobf build or whatever. Some mods seem to work, when available via a maven repo, but that is rarely given. Which is just bonkers for a mod mainly designed to link other mods together. And no, resorting to System.out.print() is certainly not an option...

A port to 1.11 is probably still possible, but there are so many things left which are based on 1.7 or 1.8 and thus are really messy. So a quick port to 1.11 will certainly make it way more messy. I quickly scanned over the current attempts and there are probably many things, which might work now. But they are guaranteed to break once Forge or Vanilla changes something. Probably in a very obscure way. Combined with the lack of time to actually review any possible PR, I cannot really accept them with good conscience.

1.12 will probably be way worse. I really expect the new recipe to majorly screw with AE2. Not that I do not like the idea to have .json based recipes. It will actually be similar to what we already have. Just without us having to maintain the actual code.

But I expect that they will be mostly designed for vanilla recipes. Which will be fine for the basic recipes like the decorative blocks. But probably break for anything oredict related or more complex NBT stuff. Forge will probably provide a half cooked solution for it, which will allow some recipes to work, but still force us to write our own system. Stuff like the facade recipes, inscriber recipes which do not consume all items, etc. Thus we will end up with some recipes using the Mojang .json syntax, some Forge and some are either hardcoded or using something completely incompatible.

As we are not the only one affected by it, I highly expect to every other mod implementing their own system and of course a good amount will implement it in such a way that it totally breaks autocrafting.

A really major break will be that Lex in his infinite stupidity seems to have decided that every mod is no longer allowed to have blocks or items disabled via config. (As the config will not be loaded at the time you have to register your blocks/items)

Something AE2 highly depends on. On one side as a feature to allow mod pack makers to customize the pack. Which is now gone, every player can simply spawn in any blocks they miss, regardless of the authors balance idea. Or just simple things like disabling silicon or redundant dust/ore/ingot types. Also gone, let the player have fun dealing with 10 different copper ores...

The more serious part is that we need it in certain cases. Like once channels are disabled, we have to disable controllers and smart/dense cables as it will otherwise cause havoc. Which pretty much means we have to completely redesign the grid system. Or remove the channel option and have the vocal majority constantly whine about it. Having different implementations, say to fallback to a dummy implementation, which just adds a decorative block, is of course also not possible. Thanks to the config file not loaded. Or we have to write our own config system..

So for 1.12 I pretty much expect these requirements:

All in all pretty much touch/rewrite the most complex parts of AE2. Combined with already expecting that 1.13 will be another major break. Like there are still talks about removing metadata, which would be one option to avoid the registration part (put everything into a single itemid and just use metadata just as now. Only on a ridiculuous level).

In terms of the other port (modern one). I would not expect anything. They spent like last year to redesign the API part, which was just a fancy way to query the GameRegistry for a certain block/item/whatever and have convenient methods to create an itemstack or similar (or null when disabled). Which of course is completely wasted time with 1.12 as it no longer allows blocks/items to be disabled.

Dhvagra commented 7 years ago

luckily, recipes aren't that horrible in 1.12 forge.

and those jsons not being horrible can play well into autocrafting.

yueh commented 7 years ago

Sure the basic recipes are pretty easy. But none of them have NBT data or more requirements.

Like the facade recipe is just any itemstack + 4 anchors. But it has to meet some requirements like

any is probably already outside the normal features. Maybe the factories can solve it. But then it is forge. Thus half is NYI and the other half is useless. At least for any sane solution. Like I seriously do not want to have to mock a fake world as it is no longer an ItemBlock and we actually have to "use" the item on a mock BlockPos to observe, if it could place a block. Which then blows up because they expect a real world as they need the chunk coords for tinting the block..

You can bet something is missing in forge and we have to find a workaround, probably a risky one which could cause a player to loose their inventories due to unable to detect full or empty cells or something similar or open the can of worms and have to implement a custom solution.

Dhvagra commented 7 years ago

well, i'm sure there's people kind enough to help :)

JelmarG commented 7 years ago

So that basically means AE2 is dead?

A rewrite is a noble idea, of course, but if there isn't a port that works in the meantime, the mod will probably be obsolete by the time the rewrite is done. That would be a massive shame of such a fantastic mod.

In other words, wouldn't it be a better idea to provide a working 1.11.2 port, and then think about rewriting parts of the mod? It seems to me that it's a bit premature to think about all the possible ways 1.12, 1.13 or forge can break AE2 in the future. Isn't it a matter of crossing those bridges when we get to them, and work with what there is at this moment and at least make it so players can enjoy AE2 in 1.11.2 playthroughs?

falizure commented 7 years ago

i'm still waiting on full 1.10 support, even some passive remarks to current issues would be nice other then just silence. If ae2 gets dropped in the future fine ill figure something else out but in the mean time its a central feature in many packs still, at least those that don't used refined storage (which at this point is tempting even for me)

thiakil commented 7 years ago

Like I am no longer able to debug AE indev with any other mod present as normal .jar as the classloader completely screws up.

@yueh Can you elaborate on that? I've had no (unsolvable) issues in dev environments with other mods & AE2

But they are guaranteed to break once Forge or Vanilla changes something.

In other news water is wet...? 1.11 should be in non-breaking changes stage, but yes it's natural if something changes in the core code things might break...

A really major break will be that Lex in his infinite stupidity seems to have decided that every mod is no longer allowed to have blocks or items disabled via config. (As the config will not be loaded at the time you have to register your blocks/items)

Just because they're registered, doesn't mean there has to be a way to craft it (i.e. don't register the recipe, have there be a condition that depends on config, etc). If someone cheats it in, well that's on them. Otherwise you're trying to 'balance' creative, which seems a fool's errand to me.

@JelmarG

So that basically means AE2 is dead?

Only in the sense that there won't be a release from the original team due to their personal time constraints. As it is open source others (myself being one of them) are working on "unofficial" ports independently which by the terms of the licence should mean that the original team can integrate that code in the future should they so desire.

As for 1.12, once forge is stable regarding recipes it will be looked into and dealt with by someone in the community, I'm sure.

JelmarG commented 7 years ago

@thiakil You and @GuntherDW could team up. His 1.11 port is pretty much done. I'm sure you guys can get a solid port going on.

thiakil commented 7 years ago

I'm sure we will eventually. Right now my port is actually further along, just not yet public.

GuntherDW commented 7 years ago

@thiakil could you define that "further along" a bit more? My first point was getting a base up and running, IE a version people could actually start playing on ready. Any and all "improvements" or other "fixes" would come later.

Besides the rendering thing which I'm having issues with (This is the first time I'm actually doing any sort of rendering code with forge so it's a bit "experimental" for me), what else is there still to do? QNB's work, P2P's work. I could re-enable or add the WAILA/IC2 interfaces, other power networks, and add the fixes that are in the 1.10 version (in fact my "dirty" repo had just that), but IMO getting a version up and running for 1.11.2 was of more importance to me in the end. Once I've figured out what exactly was changed or is causing the issue it's done. Time to "release" and start implementing the rest of the 1.10 stuff and bugfixes/improvements/features/....

thiakil commented 7 years ago

That rendering error is fixed, along with 2 others. Also fixed an integration issue. I don't have my repo accessible right now to bother listing them all.

JelmarG commented 7 years ago

Can't you guys combine forces? Right now it sounds a bit like it's a competition. ;) I know for a fact that GuntherDW has been working on his port for quite a while and that it pretty much works. It's a bit a waste of time to both work on the same thing.

GuntherDW commented 7 years ago

Well I just found out what caused the rendering issue by diff'ing the 1.10 files with the 1.11 files. Looks like it was something that yueh comitted... I don't know why I didn't do this sooner.

thiakil commented 7 years ago

I don't treat it as a competition, I started mine without knowing about Gunther's. I simply work differently and haven't pushed my commits worked on this week yet

yueh commented 7 years ago

@yueh Can you elaborate on that? I've had no (unsolvable) issues in dev environments with other mods & AE2

I have not really an idea what is causing it. Just that the classloader or asm patcher is unable to load/patch classes when launched in debug. And then failing with a ton of ClassNotFoundException. Launching it via a gradle task is totaly fine, but they are not debugable. It is just a fancy way for gradle build && cp build/libs/*.jar /path/to/instance/mods && minecraft launcher. So completely unusable.

In other news water is wet...? 1.11 should be in non-breaking changes stage, but yes it's natural if something changes in the core code things might break...

This is not about forge/vanilla breaking something during with a new major version. These are intentionally caused by violating the contract or at least intention of something because "but it works". Like using a randon function in forge, which clearly states returns 1 for negative numbers, -1 for positive, but happens to use Math.signum(), which you need. So instead of simply using -Math.signum(), you are using the forge wrapper and then start to whine, once they change it to return i < 0 ? 1 : -1 for performance reason, while still maintaining the original contract.

Just because they're registered, doesn't mean there has to be a way to craft it (i.e. don't register the recipe, have there be a condition that depends on config, etc). If someone cheats it in, well that's on them. Otherwise you're trying to 'balance' creative, which seems a fool's errand to me.

So you find it ok, if the game crashes in creative mod just because you placed a random block? Also there are still things like minetweaker/crafttweaker or others. Should we crash on launch when detecting one of them because they could allow to add a recipe for a disabled item? Wait.. even minecraft has a custom recipe system... should we crash when detecting the mod being loaded in a minecraft environment?

Only in the sense that there won't be a release from the original team due to their personal time constraints. As it is open source others (myself being one of them) are working on "unofficial" ports independently which by the terms of the licence should mean that the original team can integrate that code in the future should they so desire.

While the code is technically open source, there are further limitations for the assets. Which for example forbid any monetisation. So the obvious ones are you cannot participate in the curse reward program, use ads in any way or accept donations for developing it. But these are just the easy ones. There is also a good chance that hosting it for example on curse might violate it, because curse itself uses it commercially. But a fork will not be able to give curse a license for it. At least not without new textures, models or other assets. Which also cannot be based at all on the current ones.

So while it sounds nice on paper, actually forking it and also being able to distribute it might be tricky. Like you could use the github releases or your own website (as long as no ads, sponsoring etc), or simply build from source. But that is certainly very inconvenient for any actual player as no modpack could really include it without having the players manually download AE2.

In other words, wouldn't it be a better idea to provide a working 1.11.2 port, and then think about rewriting parts of the mod? It seems to me that it's a bit premature to think about all the possible ways 1.12, 1.13 or forge can break AE2 in the future.

1.12 is already released and most mods will start moving towards it. Thus there will be not many reasons for a 1.11.2 modpack. It is really not ideal with having the community split into 4 versions (1.7, 1.10, 1.11, 1.12). As like each one is missing some mods, so you cannot simply move your pack from 1.10 to 1.11 without losing some mods.

Isn't it a matter of crossing those bridges when we get to them, and work with what there is at this moment and at least make it so players can enjoy AE2 in 1.11.2 playthroughs?

I hear that a lot. But it usually means "let's run away from the bridge as long as possible". Of course long enough so the original bridge no longer exist and you have to build your own. While it is true, that planning for 1.13 is pretty useless, at least we should get rid of the stuff, which is just 1.7.10 ballast and dragging us down. Sure we could make a direct port to 1.11 solve all the fixes and then skip straight to a direct port to 1.12 (which is already released) and fix everything with workarounds again. Once that is done, 1.13 is probably near its release and so on. At some point we have to stop and rewrite stuff in a major way. Otherwise it will sink the mod at some point.

The main problem is that we simply lack man power. Sure there are people around, who claim they want to help. But it usually dries out once they realise is not like 20 LoC and we also have to enforce some requirements. Like not pulling every change someone proposes. While fixing their issue/feature, it will break every other existing solution. Which is simply not an option.

JelmarG commented 7 years ago

I'm sure there are enough people who would be willing to to help, even if it's a lot of work. But if there isn't a central figure within the owner group of the repo to actually lead the endeavour, there will never be enough manpower. That'll probably sink the mod before anything else.

thiakil commented 7 years ago

And then failing with a ton of ClassNotFoundException

Very likely means that for non-deobfed mods that they aren't handling the Forge runtime deobf (for modern Forge anyway). Or, in the case of deobf builds that the mcp mappings used were a different snapshot, which the deobfCompile dependency handles and needs to manually be done for non-maven deps.

So instead of simply using -Math.signum(), you are using the forge wrapper and then start to whine, once they change it to return i < 0 ? 1 : -1 for performance reason, while still maintaining the original contract.

This specific example sounds like a failure in the coder using the function. If you need Math.Signum, why not use it? Unless the wrapper method mentions in its javadoc the exact same output for positive/negative zero, you cannot depend on it being the same as a function it so happens to call in the version where you looked at the code.

So you find it ok, if the game crashes in creative mod just because you placed a random block?

What on Earth would be causing a crash in this instance?! If you block is registered like it is supposed to be there would be no Forge crash. Are you suggesting AE throw an exception if a block disabled in config is placed? That would be a dick move.

Which for example forbid any monetisation. So the obvious ones are you cannot participate in the curse reward program, use ads in any way or accept donations for developing it.

Not explicitly, no. The CC NonCommercial clause prohibits uses that are "primarily intended for or directed toward commercial advantage or monetary compensation."

Granted, it does say that's essentially up to the rights holder to decide, but do you really consider being part of a version port mod a primary intention for monetary compensation? If I took those textures and put them in a resource pack targeting another storage mod, and then gained money from that, then yeah I would consider your rights violated. Compensation for effort put into porting and maintaining a mod would be the primary purpose in AE2 as a whole's case.

Also at the end of the day it's pretty much a non-issue unless you, another member of the original team, or the texture creator decide to pursue action.

1.12 is already released and most mods will start moving towards it. ... snip

Largely this is a philosophical issue, some people prefer jumping straight to the latest version and dealing with everything at once, others find benefit in doing it incrementally and having extra version support as a positive. To each their own. Same with the running away from the bridge part.

at least we should get rid of the stuff, which is just 1.7.10 ballast and dragging us down.

Is this listed somewhere? Can it please be? Sounds like useful information to me.

electrofloat commented 7 years ago

Wow, I didn't expect yueh describe the issue so extensively, but it is very useful to know what he thinks about the project. Although as I see he mostly wrote about problems and not solutions.

I can also understand while a lot of people saying they could help with the port, why yueh thinks they will dry out quickly. What I think is somewhat inline with JelmarG. There needs to be someone who conducts people what should be/shouldn't be done. If yueh doesn't have the time for this, he should let someone else do it. (someone else with push/PR accept permissions?) (If I knew Java I would also help with the rewrites, but fortunately (in this case un*) I only know C++ well enough))

yueh commented 7 years ago

I'm sure there are enough people who would be willing to to help, even if it's a lot of work. But if there isn't a central figure within the owner group of the repo to actually lead the endeavour, there will never be enough manpower. That'll probably sink the mod before anything else. Wow, I didn't expect yueh describe the issue so extensively, but it is very useful to know what he thinks about the project. Although as I see he mostly wrote about problems and not solutions.

Probably applies to both.

The solutions are pretty limited. I could probably dedicate some time for AE2, but more towards project management. Say issue management, development roadmap, "mentoring" and so on. Maybe a small bugfix here and there or code review. But that is pretty useless without any real development. Bit of a chicken/egg problem. Without a lead, there won't be many changes. But without devs, there is also nothing to lead.

Also having a high dev turnover rate is also not helpful. I certainly cannot spent a month explaining the internals to some new dev, just to have them leave 1 month later because some reasons.

Very likely means that for non-deobfed mods that they aren't handling the Forge runtime deobf (for modern Forge anyway). Or, in the case of deobf builds that the mcp mappings used were a different snapshot, which the deobfCompile dependency handles and needs to manually be done for non-maven deps.

I already considered it. But neither normal jars nor deobf work. Regardless of matching mappings or not. I even build custom deobf version with matching forge versions/mappings or changed AE itself. Etc.

The moment it runs via debug. The IDE probably uses a real javaagent to inject debug/sampling/whatever and then fucks with the forge homebrew class loader. It kinda works with largly forge independent mods (JEI for example). But the moment it touches blocks and forge wants to asm patch the crap out of them it fails to actually patch them and dropping the class completely. I also talked with mezz, asie, some others and iirc cpw. But even they have no idea.

Probably started around the end of 2016 or beginning of 2017. So it not always happening. Or at least I did not run into it previously. Like maybe it would have affected it previously, but just a reduced amount of mods. Until forge decided to asm more stuff. Like similar to the fluid breaking stuff during 1.7.10, where they asm patches old mods in the hope they will then work with the new system.

This specific example sounds like a failure in the coder using the function.

And that applies to some of the forks on a quick glance. Like some comparisons against Items.AIR (isn't that null?), when it should probably be itemStack.isEmpty(). If mojang or forge has the glorious idea to make air -1 it will probably explode in some rare edge cases. And then you have to ask yourself does it crash because AIR is no longer null and you should fix it to still consider it as AIR or was the port faulty and it should actually be isEmpty().

What on Earth would be causing a crash in this instance?! If you block is registered like it is supposed to be there would be no Forge crash. Are you suggesting AE throw an exception if a block disabled in config is placed? That would be a dick move.

It kinda applies to channels/controllers. Disabling channels will also disable the controller as well as smart/dense cables. Which is required because it does not actually disable it, but remove the upper limit. With that in mind, cables could transfer thousands of channels, but the rendering etc is still expecting at max 8/32 to select the right textures. So keeping the cables is not really an option. While we could simply use Math.min(amount, 8) or so, it will still be confusing to display any value between 0-8/32 even with disabled channels.

Something similar applies to a controller, they are more like a flag to a grid and the moment they are present, it will fall into a controller based grid. Not an adhoc network with a limit of 8 channels (or infinite with disabled, but still an adhoc). So the moment a player places them it will also not longer work with disabled channels and fallback to a normal grid.

It would be an option to have like multiple tile entity classes and just replace the normal one with a decorative dummy one, which does not count as flag. But thanks to forge, we have to register the stuff before knowing channels could be disabled...

Not explicitly, no. The CC NonCommercial clause prohibits uses that are "primarily intended for or directed toward commercial advantage or monetary compensation."

The CC NC is really vague here. It really depends on who you ask or what you read and goes from "yeah, you can use as long as you don't sell it directly" to "You cannot make a single cent, even indirectly".

With Curse being actually amazon now, I have no idea how they would manage it. At some point another mod might decide to fuck with them. Which then causes amazon to simply ban everything based on CC NC as a precaution.

Is this listed somewhere? Can it please be? Sounds like useful information to me.

Pretty much the list in my first post. But to summarize it shortly

thiakil commented 7 years ago

And that applies to some of the forks on a quick glance. Like some comparisons against Items.AIR

Indeed, I see no need to compare to AIR, should be handled by IS.isEmpty() anyway

Cable renders - I'd be setting it to 0 and display as if no channels were going through it.

Controller / Grid type - I imagine the check for a controller would need to be adapted to stay in ad-hoc when channels disabled.

Unless it has changed since I last looked, the TEs don't have to be rendered (EDIT: registered) in the Block/Item events, should still be possible to do after config loaded.

GuntherDW commented 7 years ago

About the checks against AIR, I should indeed redo those checks, or rework them. I was planning on redoing the inventory stuff indeed anyway. It's a garbled mess at best.

Rendering is not my forte (ATM), so I'll probably leave that to other people.

But as I stated in a previous commit. My first intention was getting something up and running, after which we can "clean up" or even do some required rewrites whilst the playerbase at least has something to play with. That might lead to ugly code, but I'd rather have a happy playerbase than one that has to wait for 5 months to get something released because there's no dev around to rewrite 75% of the mod.

The debugging is also quite weird. I'm using IntelliJ myself (community version to be precise), and have had no real issues with debugging. Other than the bytecode not matching the source sometimes at least.

electrofloat commented 7 years ago

@Ommina http://www.minecraftforge.net/forum/topic/58991-forge-14211-minecraft-112/ Those guys are working fast these days.

thiakil commented 7 years ago

@GuntherDW there is a difference between a rewrite to solve long standing issues, and needing a rewrite because you ported things incorrectly. The point is that you shouldn't have needed to check for AIR in the first place, as you're just asking for hidden bugs that are going to pop up for you later - for example AIR is not the only thing that makes a stack empty.

And that's just one example of bad band-aid code. In another instance you changed a core Platform method to make something that uses it work, instead of fixing the actual problem.

thiakil commented 7 years ago

@yueh good news for the config discussion earlier (added emphasis mine)

Modders: It is now recommended and HIGHLY encouraged that you use the RegistryEvent.Register functions when creating your block, items, potions, recipes, etc... These events have changed to being fired after pre-init.

GuntherDW commented 7 years ago

I know that "Items.AIR" is not the only thing that makes a stack represent itself as "empty". I also know that a couple methods and "band-aids" that I applied are bad. That's why I tagged them as such. They are not meant to stay like that.

I only did that to get something up and running. That "Platform" change is the first thing I did in the codebase before I even got something up and running, I was planning on going back on a couple of the things that I hastily did to fix it up correctly.

Also, the rewrite is not only because I ported stuff incorrectly, it's as yueh said clear as day that big parts are in need of a rewrite because they got patched up continuously instead of receiving the care they needed. The "AIR" thing is because my IDE was complaining on it never being null, and a quick glance at the code that it was calling showed that it, like an ItemStack, could never reach "null", but gets a default value assigned as well. I am far from finished on those regards.

I should maybe not have committed everything, in spite of some prying eyes blaming everything on some hastily done code that's probably get "fixed" in the end. I should maybe have kept my stuff hidden as well then, or at least not in such great volume as this.

What really saddens me though is the vitriol towards each other instead of trying to get AE2 back up and running in a playable state.

thiakil commented 7 years ago

Hasty fixes are what leads to a mod getting a reputation for being buggy and unstable (and there will always be someone who gets that impression even from an unofficial release from Github). Github is a form of peer review, so yes your code is going to be critiqued, especially if you're offering it up for people to actually use as a release.

yueh commented 7 years ago

In some cases it might actually have to be compared against AIR and not isEmpty(). Technically there might be no difference, but it has a different semantic. That is certainly a small advantage with 1.11. But that only applies once it is ported correctly.

Also I would not ignore the IDE doing bullshit. Especially with Intellij, we are at a point where the IDE thinks it is better than the developer and starts doing all sorts of stupid things. Like reformating a string (say block id) from 'ae2.blocks.foobar' to "ae2 . blocks . foobar" because it likes to put spaces around dots... Thus I would not be surprisedm should it suggest AIR as replacement for null in certain cases.

Regarding the "vitriol", that is certainly not the case. Nobody will write perfect code and therefore at some point will commit something stupid. Of course the port is not done. But it is a pretty large amount of work, so reviewing it once it is near completing is way more work than some incremental feedback here and there. Better have someone say it is really stupid, even if it might hurt someones feelings, than staying silent and watch it break.

yueh commented 7 years ago

Quick update as I had a bit time to look over the forks.

GuntherDW's is pretty much a straight port to 1.11. There might be a few bugs here and there or maybe a bit too quick'n'dirty. But these should be easy to iron out.

thiakil's is nearly at a point, where merging it back is no longer an option. It reintroduces already fixed bugs. Removes code we actually need for various reasons, like forge being lackluster or it might simply not be obvious why it is needed. Yes it could be simply copied back, but that destroys the whole history. Which is already a nightmare because it happened when AE2 went open source and reconstructing a specific case and why or how something was fixed/changed/etc (or not) is very tedious. Further it tries to add features back, which we deliberately hold back in anticipation of Lex fucking with coremods. Which of course did not happen in 1.11, but now in 1.12. Especially with FTB now being even more restrictive as forge itself. Ultimately it will result making a port to 1.12 even harder as we might not be able port these features at all.

electrofloat commented 7 years ago

FYI: There was a 'Townhall' meeting on 1.12 /forge/coremods with Lex, cpw, and others, on FTB's official twitch channel. You can watch the whole thing here: https://www.twitch.tv/videos/155878312 Unfortunately a bit long and no transscripts yet.

thiakil commented 7 years ago

It reintroduces already fixed bugs.

Could you give an example?

Removes code we actually need for various reasons,

The update checker is the only thing I can think of that I straight up removed.

As for for "tries to add features back, which we deliberately hold back in anticipation of Lex fucking with coremods", which I can only assume refers to the integrations, only a couple of small parts of that actually NEED the coremod, and even then, only because of how it's coded.

electrofloat commented 7 years ago

@yueh don't blame the guys for adding/removing things they don't know they shouldn't have.

Instead let them know what is the direction/what is the condition to accept a pull request, also document those "various reasons" for not removing/removing code, because noone can see inside your head to find out why they shouldn't have done what they've done.

Make a development section on the wiki or something and document! Write everything down you have in your head about the project.

yueh commented 7 years ago

@mydexterid I certainly won't waste nearly 2 hours to watch them ramble about some useless stuff. If they are not able to write the requirements down as text, I pretty much don't care and simply consider it as "core mods banned". Yes, I know you can bundle them as .jar inside your .jar. But they need to be signed and what not. Which the current build server does not support. Changing it might not be possible or if it could take a long time. Replacing it completely is of course also an option. But that also requires a new website/wiki or a better ingame documentation (like the botania book). Nothing which can be done in an hour.

Could you give an example?

Off the top of my head some rendering related stuff like flipped textures. Because the obvious fixes were already tested and dropped because correct rotated facades means incorrectly flipped textures on some parts of cables. That is why I listed it as needing a rewrite. I would not be surprised, if the original code is from AE1, which included all weird 1.5/1.6 behaviour. Which then is wrapped somehow to accommodate to counteract rendering changes/fixes in 1.7 and so on. I am just making that up, but imagine something like 1.5 having a bug that the top texture was always mirrored, because the vertices were ordered backwards, so AE1 had a workaround for 1.5. 1.6 then fixed it but now had it rotated by 180 degree instead of mirror. So it was wrapped somehow to mirror it twice and rotate it by 180 degree again. Then add whatever you can imagine the port to 1.8/1.8.9 did break or add to it.

The update checker is the only thing I can think of that I straight up removed.

That (because we need it as the forge one will not work). Also API things like the MEMonitorThingy. It might still be needed for addons. Therefore it was still part of the API as long as there is no alternative.

As for for "tries to add features back, which we deliberately hold back in anticipation of Lex fucking with coremods", which I can only assume refers to the integrations, only a couple of small parts of that actually NEED the coremod, and even then, only because of how it's coded.

Pretty much all integrations, which are not based on caps. So OC tunnels (which where always a bit broken). Or the IC2 tunnels (these might actually work without coremod or optional, but the whole P2P design does not really fit perfectly). Also pretty much all wrench integrations. thirdparty needs to go away. A public maven repo is a requirement to be considerd as integration. Ideally with an api artifact to avoid polluting the dev enviroment, increasing loadtimes and so on. (And this also includes no ivy workarounds pointing to a fixed curse forge url or similar)

@yueh don't blame the guys for adding/removing things they don't know they shouldn't have.

Instead let them know what is the direction/what is the condition to accept a pull request, also document those "various reasons" for not removing/removing code, because noone can see inside your head to find out why they shouldn't have done what they've done.

Make a development section on the wiki or something and document! Write everything down you have in your head about the project.

This would pretty much be the perfect approach. But it takes time I do not really want to waste, should nobody actually decide to contribute something. Ideally continuously and not a one time port. Of course I cannot expect it, reallife can always change and make it hard to continue. But AE2 is also not a project, which can cycle through a new "intern" every 2-3 months. It is a bit too complex for it. Or at least not in a state that you can easily hand to someone, who might not have the knowledge about AE2, but is firm enough with forge/minecraft to maintain decorative blocks or simple tools. Maybe also add new ones, if there is a need (and we actually have someone for textures).

I can also understand, should someone not want to work under someone, who just makes up the user stories and hands it off to them for the actual work. I personally do not like the idea that much considering it is still a pet project, but it is probably necessary at least in the beginning to give a direction and keep the design ideas behind AE2.

As I didn't get any response to offering some free time for project management, which a wiki, better contribution guidelines and so on could include, I can pretty much only watch 2 (or maybe more forks) competing instead of combining their resources and judge a potential merge candiate on some some internal requirements, goals, etc.

electrofloat commented 7 years ago

@yueh There are of course written requrements here: http://www.minecraftforge.net/forum/topic/58706-regarding-minecraft-112-and-policy-changes/ They were just discussing this in the video I've linked, which you can listen only to like a podcast.

Regarding project management/wiki. First and foremost I think written guidelines would be much appreciated, and the wiki is a good place for it. There were numerous useful and detailed info in this issue and I remember other issues containing other bits and pieces of info. So all of these should go on a wiki. If you just dump it on I can make sure to sort it out if you don't have time for it. But it first needs those text to be written and it seems noone can do this but you.

And also please don't feel like you're wasting your time documenting this project. There will always be people interested in AE2 and this will make those people's life much-much easier.

thiakil commented 7 years ago

Indeed I messed up the UVs initially, but that was fixed in a later commit that made it do the different stuff only for facades.

Facades use a baked model, from the new rendering system. Currently only lacking caching, which is pretty trivial.

What won't work about the forge updater? All it needs is a json file, which can be loaded via github. Which is exactly what I have done for my fork. I even wrote a gradle task to generate it.

I moved MEMonitor because it was marked to be moved and rv5 is a new api version and as such I believe api breaking changes are fine. Sure, you might not want to reintegrate that, but I got the impression it was unlikely to happen even if you did like the code.

Yes the p2p tunnels require a bit of modification, however forge can strip interfaces as far as I know, and the layers don't need to be added if the integration is disabled. Nb: no I haven't looked much into this yet, as I have been focussed on other stuff, so lack of knowledge may show. Coremod can stay for as long as it needs to. I can setup my dev server to do whatever I want.

I use gradle deps whenever possible. I didn't check if build craft had one no, but that's only one file, and I personally don't see the problem. Cofh has to be done via gradle for 1.12 by their rules.

As for the project management comment, I think it got lost in all the rest of what we were talking about because it didn't sound definite that you could spare the time. Thats just my view on it anyway.

yueh commented 7 years ago

Indeed I messed up the UVs initially, but that was fixed in a later commit that made it do the different stuff only for facades.

I have yet to see a working solution. One of the main problems are the inset parts of smart/dense cables. On some sides they are flipped with pretty much every fix we have tried before. I am pretty much at the point blaming minecraft itself for it. Like instead of rendering the inset part, it is actually a qube with a transparent side and we see the backside rendered, therefore the textures are actually flipped.

Facades use a baked model, from the new rendering system. Currently only lacking caching, which is pretty trivial.

We had a couple of ideas for facades. Like stop rendering the 2px sides as the block texture. Which should make it way easier to render blocks with multiple texture passes (which is not that trivial to render on a non full side). Cache probably needs some testing, no idea if the memory consumption is worth it as it should only be needed on chunk updates and it could easily contain severel hundred of blocks.

What won't work about the forge updater? All it needs is a json file, which can be loaded via github. Which is exactly what I have done for my fork. I even wrote a gradle task to generate it.

It simply cannot compare the version. So it could report an older version as "new". Or not work at all. So pretty much useless when the only use is to show if there is an update, but actually failing at it. Maybe it is possible to integrate the result somehow into the forge one instead of printing it to the log or integrate into other updater mods.

Yes the p2p tunnels require a bit of modification, however forge can strip interfaces as far as I know

This is extremely limited and can easily break. Like EnderIO pushed a RF beta release in 1.7.10 as release, thus forcing everyone to update their mods and release a new version before every player could resume playing as it crashed constantly. Our system at least allows it to be deactivated in the config, so it does not stop you from playing. It also gives a modpack maker the option to restrict for example the energy type a player can use to power RF. Like maybe they want to force a player to use IC2 reactors as only power source for AE, while still having some RF mods. Of course you can wrap every method into an if(integrationsX.isEnabled()) and try to provide a disabled implementation, but that might be not possible. Like you could still have a X P2P tunnel and it will render as sucessfully connected, but simply does not work, because the only condition to render a connection is an instanceof X

I use gradle deps whenever possible. I didn't check if build craft had one no, but that's only one file, and I personally don't see the problem. Cofh has to be done via gradle for 1.12 by their rules.

One reason is simply that ForgeGradle does not handle multiple sourcesets that well. So anytime you rebuild the project, it messes with the IDE config and you have to manually fix it. So pretty annoying, even with a single file.

As for the project management comment, I think it got lost in all the rest of what we were talking about because it didn't sound definite that you could spare the time. Thats just my view on it anyway.

As said, it kinda depends on the interest for it. I won't be able to guarantee I can spend a fixed amount per day/week/month on it or that I am able to give you feedback on the same day/hour. It might take a few days or maybe a week depending on how complex it is.

thiakil commented 7 years ago

I have yet to see a working solution. One of the main problems are the inset parts of smart/dense cables.

My solution seems to work at least for the tested Bookshelf block. Wouldn't surprise me if there are still edge cases though, I haven't tested absolutely every block. Bookshelves were simply the easiest due to the asymmetry.

Cache probably needs some testing, no idea if the memory consumption is worth it as it should only be needed on chunk updates and it could easily contain severel hundred of blocks.

Currently it's marked in the source as a TODO, and sounds like a good idea to me (its basically the point of the baked model system). At the very least caching blockstate+side -> texture would prevent having to do an extra quad generation (to use getQuads) each time.

It simply cannot compare the version.

I beg to differ. [Forge Version Check/INFO] [ForgeVersionCheck]: [appliedenergistics2] Found status: OUTDATED Target: rv5-alpha-1 ComparableVersion tokenises the version and will compare integer values. String values must match exactly. Indeed it could fail if you had say different RV versions on the one MC version, but that is currently not the case. Should that become an issue codenames instead of rvX would be an easy solution.

This is extremely limited and can easily break.

You would only need to strip them if the interfaces don't exist. For cable bus, the layers simply need not be added (e.g. when I had neglected to re-enable the IC2 layer, the cable did not connect), and nothing should load them. As for AERootPoweredTile, RF would need to be refactored, possibly like what is done to the cable bus layers. I can't see where IC2 power is externally exposed externally either.

As for that RedstoneFlux class, I'm not even sure it's needed, as FE is widely supported, even by TE/TF itself.

Like you could still have a X P2P tunnel and it will render as successfully connected, but simply does not work, because the only condition to render a connection is an instanceof X

Completely incorrect, if the integration is not enabled, the layer is not added, and it is not an instanceof. Additionally the tunnel item/part should not exist or otherwise throw an exception when it is created.

One reason is simply that ForgeGradle does not handle multiple sourcesets that well. So anytime you rebuild the project, it messes with the IDE config and you have to manually fix it. So pretty annoying, even with a single file.

I'm not exactly sure what you're getting at here, never experienced such a thing myself. The only issue I have with adding the files to git (either directly or as a submodule) is that they can be a pain to update. But thankfully they don't really change often. I've been meaning to ask KL if he'll make a pre-1.12 api jar too.

As said, it kinda depends on the interest for it. I won't be able to guarantee I can spend a fixed amount per day/week/month on it or that I am able to give you feedback on the same day/hour. It might take a few days or maybe a week depending on how complex it is.

I'm happy either way honestly; I don't mind handling all that myself - even if that means requiring a fork. If users don't like it being a fork and interest drops in favour of an 'official' copy, then so be it. I also certainly don't expect to create a pull-request with my branch as-is, that would indeed be silly as I've specifically marked a couple of things as not suitable for reintegration.

yueh commented 7 years ago

Currently it's marked in the source as a TODO, and sounds like a good idea to me (its basically the point of the baked model system). At the very least caching blockstate+side -> texture would prevent having to do an extra quad generation (to use getQuads) each time.

TODOs are not "this must be done exactly as written". Instead these are mostly ideas that these things might be nice, but need further evaluation.

I beg to differ. [Forge Version Check/INFO] [ForgeVersionCheck]: [appliedenergistics2] Found status: OUTDATED Target: rv5-alpha-1 ComparableVersion tokenises the version and will compare integer values. String values must match exactly. Indeed it could fail if you had say different RV versions on the one MC version, but that is currently not the case. Should that become an issue codenames instead of rvX would be an easy solution.

You cannot pick the one case, where it does not fail. There are tests for it, which need to succeed. If not, it is simply broken. I would even consider the tests being incomplete in regards of testing against a completely broken implementation. Cases like 2 > 10 are missing and I would not be surprised should forge fail here, too.

It is the usual example of Forge lacking any foresight. Instead of also providing a way to register different version schemes. Either by allowing to point to a class to reflect it (meh), or have a version scheme registry so you can register them. Which would also solve issues regarding the mod dependencies on specific version ranges. Instead the fallback to the usual "I do not see any need for something different than semver". Even when most mods do not even use semver, but still try to enforce it. While the sentence before did state "We do not want to enforce any standards, we just consider the project as some sort of abstraction layer on top of Minecraft, not a real library for making it easier to develop mods"

Integrations/Tunnels.

This is not only about tunnels. These are way more forgiving towards missing dependencies, stripping and so on. It is mostly about the full support. Like we can easily add RF tunnels, even without using our coremod. But we cannot add RF to power AE itself. So we would end up with a RF tunnel, but the player is not able to use RF to power an energy acceptor. There is no way to explain that to a normal player.

Further the it comes with massive downsides. Like not being able to debug anything related to tunnels. It is just guessing or debugging it on a piece of paper and then completely missing something obvious, which would immediately pop up when debugging it with an IDE.

thiakil commented 7 years ago

TODOs are not "this must be done exactly as written". Instead these are mostly ideas that these things might be nice, but need further evaluation.

And I'm of the opinion it should be done. You seem not convinced, that's fine.

You cannot pick the one case, where it does not fail.

I can't fault that. Though I'm struggling to understand where this failure state you're certain of comes from. Which tests exactly are you referring to?

But we cannot add RF to power AE itself. So we would end up with a RF tunnel, but the player is not able to use RF to power an energy acceptor.

I'm going to assume here you mean "whilst retaining the ability to disable with config and the only other option acceptable to me is to omit it entirely" (please do correct me if I'm mistaken) as otherwise it doesn't make sense due to Forge's own stripping avenues. Your statement only holds true if AE is not modified to accommodate the no coremod state.

yueh commented 7 years ago

I can't fault that. Though I'm struggling to understand where this failure state you're certain of comes from. Which tests exactly are you referring to?

The unittests you deleted, that might be the reason you don't find them. For example it must follow rules like rv5-stable-1 > rv5-beta-10 > rv5-beta-2 > rv6-alpha-0. If it just falls back to a string comparison because it is neither a semver nor an integer, it might do the usual char by char comparsion, thus rv5-beta-10 < rv5-beta-2 or report a rv6-alpha as update for a rv5-stable, which should never happen. Except when subscribed to an alpha or beta channel. And no, assuming there will only be a single rv per minecraft release is false. Technically it should still be rv4 for 1.11 or even 1.12, while we could easily have rv4 and a rv5 for 1.12, while also having a rv5 and rv6 for 1.12.4. rv just indicates the API version and as long as there is no beta release for one, we can break it at any time.

I'm going to assume here you mean "whilst retaining the ability to disable with config and the only other option acceptable to me is to omit it entirely" (please do correct me if I'm mistaken) as otherwise it doesn't make sense due to Forge's own stripping avenues. Your statement only holds true if AE is not modified to accommodate the no coremod state.

Requirements are simply:

thiakil commented 7 years ago

The unittests you deleted, that might be the reason you don't find them.

Weren't we talking about forge's version check not working? Those tests test your version checker.

thiakil commented 7 years ago

If it just falls back to a string comparison because it is neither a semver nor an integer, it might do the usual char by char comparsion

It compare integers as integers - if it didn't Semver would not parse, as last I knew 1.2.1 < 1.12.1

electrofloat commented 7 years ago

@yueh That requirements you just wrote. Now that should be on that Wiki page I mentioned.