Structure changes

[Elix-x]

Followup of naming convention changes (https://github.com/AEModernMCPort/Applied-Energistics-2/issues/46#issuecomment-241037526).

Structure of AE should be decided, and more specifically, modularity. So we can either go with:

1. No modularity (current).
2. Internal single mod modularity (everything is split into modules, modules are loaded from core module, single jar).
3. Internal child mod modularity (everything is split into modules, modules are child mods themselves requiring core mod, single jar).
4. External child mod modularity (everything is split into modules, modules are child mods themselves requiring core mod, each module can be in a separate jar).

Logically, for more modularity we go, the more time it will take for us to transition current structure.

[yueh]

Note that this is in no way a final decision and it is in general something which can be changed during the development as long as we do not entrench on one option.

The only requirement I currently see is 1 or 2 layers are missing in the current package hierarchy. There is really no way currently to do something like appeng.{ core, common.{ blocks, tiles, items }, services, decorative, client, server }. Everything sits directly below appeng with packaging belonging together (blocks, items, tiles, parts and similar) and completely unrelated things like the services for update checking, exporting items and such.

Decorative blocks or tools are simply a good guinea pig to separate into their own module without any real dependencies. No fancy rotation required, some metadata based ones are more than enough. The quartz tools are more or less just vanilla iron tools. Moving them to their own module or child mod should not cause many issues and can even be done iteratively. Separate into their own packages/modules. Once they are finish, it can or cannot be put into child mod.

The only version I do not see with any benefit are external mods. Most players will not care about or just see it as tedious process when downloading. Probably even consider it as trying to achieve more downloads numbers. And it puts a huge entry point onto addons as they will have to deal with non existing mods, non working interface stripping and what not. Not just query the registry about enabled items.

Also it is easier to sell the part about "You have to make your own recipes, when not using the default config" while multiple jars clearly would announce that is planned and the recipes can deal with missing certus from the decorative jar.

[drekryan]

The only thing I can see being able to be "split off" into a module is the decorative blocks and maybe a few tools. I don't think there is even enough cases where making a module is worth it. Having just one module and the core doesn't really give any benefit. I just don't see the point unless there would be many more cases which make since to be separate but the core mod relies on many of it anyway to be functional in terms of gameplay. Just my opinion but I don't have any real strong opinions except in the case of making the modules seperate jars. I don't agree with having a separate jar for each module.

[shartte]

I think go with something between 1 and 2. As a user of AE2, I don't want to mess with individual modules. It's more mental gymnastics ("do I need this module or what?") for very little gain, given that AE2 doesn't add hundreds of fluff items anyway. Improving the package structure of the project should always be an ongoing goal. If we use static registry names (which we should do anyway to avoid accidentally breaking worlds), we can refactor very easily, given what modern Java IDEs are capable of.

I'd like to make two suggestions that would be structural changes:

• Change the mod id to "ae2".
• Make the registry names explicit. Set them in the constructor of the respective blocks / items to make it absolutly unmistakably clear where they come from.

[Elix-x]

Change the mod id to "ae2".

No. Forge does not like modids < 4 chars. But appeng2 will work.

[shartte]

@elix-x Also good, and less ambiguous than ae2, you're right.

[yueh]

If we change it to modules/mods it is pretty much:

• API
• Core
  • Config
  • Recipes
• Decorative
  • Blocks
  • Stairs
  • Slabs
• Basic Tools
  • Quartz tools
  • Grindstone
  • Misc tools like the charged staff, entropy manipulator matter cannon
• Worldgen
• ME
  • ME Network/Grid
  • Blocks/Parts
• Crafting (Reasonably large/complex to justify a module/mod)
• Server
• Client
• Misc stuff

No. Forge does not like modids < 4 chars. But appeng2 will work.

Just keep the mod id appeng, at this level it is pretty irrelevant to have a number. Pretty unlikely to ever have AE1 and AE2 at the same time. Or to annoy forge ae2.

Make the registry names explicit. Set them in the constructor of the respective blocks / items to make it absolutly unmistakably clear where they come from.

Not sure about that. Sometimes you need a overview of all and that is something current IDEs cannot really do. But the current approach is clearly a bit too heavy magic.

[shartte]

Not sure about that. Sometimes you need a overview of all and that is something current IDEs cannot really do. But the current approach is clearly a bit too heavy magic.

What do you mean by overview of all? There currently is none, really. By setting them more explicitly you gain discoverability from a source code perspective in two ways: Anyone curious can do a find-usages search for setRegistryName, since it HAS to be called at some point. Or someone can just search for the registry name they see ingame and find where it's coming from. That is not possible right now.

[drekryan]

Wait I'm kinda confused if World Gen for example as a module. How would the "core" work without such feature. If I am thinking the correct modularity here where individual modules could be enabled or disabled. Say you "disable" to the Worldgen module preventing Meteors and Certus Quartz Ore from generating. Isn't the ability the get Certus Quartz a requirement for the core mods recipes? I don't see how it would work without removing functionality that is required. Unless I am just misunderstanding.

[Elix-x]

I tottally agree. And if we split them into different mods (deco & tools & misc could be packed as misc mod),

• Core
  • API
  • Core
  • Config
  • Recipes
  • Worldgen
  • ME
  • ME Network/Grid
  • Blocks/Parts
  • Crafting (Reasonably large/complex to justify a module/mod)
  • Server
  • Client
• Decorative
  • Blocks
  • Stairs
  • Slabs
• Basic Tools
  • Quartz tools
  • Grindstone
  • Misc tools like the charged staff, entropy manipulator matter cannon
• Misc stuff

[shartte]

Having "misc" as a module is just not very good naming, sorry.

@drekryan Think in terms of skyblocks where for example worldgen is not required.

[yueh]

The API has to reach out to pretty much any module. So placing them below core is not really correct. Decorative and basic tools also depend on the recipes and config. But not on the me network or server/client.

@shartte misc is more the missing modules. Like version checker, item/block export, the debug tools.

[shartte]

Okay, that's fair enough.

Just to clarify here, are we talking packaging structure? The more interesting point there would be to decide whether we do functional or technical on the first level. Right now, it's technical (block/items/part/helpers/tile). I personally prefer functional (the user's perspective so to say) because it puts the stuff that actually interacts with each other closer together in the package structure.

Once that is decided, it becomes a lot easier to decide on the rest, although you'd still probably need some place to put actual shared infrastructure, even though one should strive to keep that part as compact as possible.

edit I would also not equate modules with packages. The first question would also be, what's a module to you. We could mean modules in the software sense, or modules that a user can toggle off/on, which toggles the functionality contained in a module on/off. Modules from a users perspective might span multiple packages.

[Elix-x]

@yueh so more like this? Wouldn't i be more convenient to have API/module?

• API
• Core
  • Core
  • Config
  • Recipes
  • Worldgen
  • ME
  • ME Network/Grid
  • Blocks/Parts
  • Crafting (Reasonably large/complex to justify a module/mod)
  • Server
  • Client
• Decorative
  • Blocks
  • Stairs
  • Slabs
• Basic Tools
  • Quartz tools
  • Grindstone
  • Misc tools like the charged staff, entropy manipulator matter cannon
• Misc stuff

@shartte We ARE moving from technical to functional. We are deciding the "depth" of move.

[drekryan]

@elix-x I like the way that structure looks. Makes more sense to me. +1

[shartte]

I am not sure about a few points, but generally agree.

API -> That should go into a separate gradle module so it can be produced as a separate jar and published to Maven easily.

Core -> Core seems redundant. I would move everything from the first Core package up one level.

Client / Server -> What do these contain? I would move rendering related stuff to where it is used. I.e. we have a special baked model used by block X, I would place it next to said block in its functional package.

[Elix-x]

Client / Server -> What do these contain?

There's no server specific stuff, but there's client specific. But it should go with the module.

I.e. we have a special baked model used by block X, I would place it next to said block in its functional package.

I don't agree on that part. Client only stuff should still be in client package (we do NOT want to load it accidentaly on server).

[shartte]

I don't agree on that part. Client only stuff should still be in client package (we do NOT want to load it accidentaly on server).

I don't think the classes being in the client package really stops anyone from using them.

[yueh]

It still has other dependency issues.

• API
  • Depends on nothing
• Core/Lib
  • Depends on API/Provides API implementation
  • Config
  • Recipes
  • Depend is a bit tricky here. Most modules depend on the recipes existing and being registered to forge. But no real code dependencies.
  • Registries
• Decorative
  • Depends on Core/Config, Core/Recipes, Core/Registries
  • Blocks
  • Stairs
  • Slabs
• Basic Tools
  • Depends on Core/Config, Core/Recipes, Core/Registries
  • Quartz tools
  • Grindstone
  • Might be a problem, because Core/Registries, Core/Recipes depend on grinding recipes.
  • Misc tools like the charged staff, entropy manipulator matter cannon
• Worldgen
  • Depends on Core/Config
• ME
  • Depends on Core/Config, Core/Recipes, Core/Registries
  • ME Network/Grid
  • Blocks/Parts
• Crafting (Reasonably large/complex to justify a module/mod)
  • Depends on Core/Config, Core/Recipes, Core/Registries, ME
• Server
  • Depends on Core/Config
  • Commands
• Client
  • Depends on Core/Config, Core/Registries
  • Probably not that straigthforward, could be a lib and the other modules depend on it. Or completely separate everything rendering related here..
• Misc stuff

API -> That should go into a separate gradle module so it can be produced as a separate jar and published to Maven easily.

It is already its own sourceset and produces a api artifact. No changes required. Splitting it completely is worse, we had it early and it is a constant struggle to maintain git submodules or real dependencies. (Change the API, build, deploy. release breaks, update release, hope nothing was forgotten when deploying the API, repeat)

Client / Server -> What do these contain? I would move rendering related stuff to where it is used. I.e. we have a special baked model used by block X, I would place it next to said block in its functional package.

See above. But server is really small, but might contain helpers for spatial dimensions or so. Client is pretty open. Depends a bit on how we package it.

What we had in mind was something like appeng.me.iface.{ Block, Part, Item, GUI, Container } or appeng.decorative.certus.{ Block, Stairs, Pillar, Dust, Seed, Crystal, PureCrystal }

Putting every rendering/client related class under appeng.client like appeng.client.me.iface.{ GUI, Helper} has certainly some advantages towards separating them. It is pretty easy to tell the static analysis that nothing outside appeng.client can depend on it. But then Minecraft itself is not really that strictly separated. It would have many packages with maybe 1-2 classes. Which pretty commonly would be edited together with the non client part.

Packing is really not that great usually. Sometimes you need everything related to the ME interface, the next case needs all GUIs together. But the later can at least be provided by the IDE. Java not really having a package system also does not help..

[shartte]

Regarding API: I am fine with the source set approach. The proposals so far didn't mention it so I wanted to make sure it's not getting moved into src/main.

Although you mentioned not liking it earlier, I'd not like to go with calling the class just "Block" since that really screws up IDE navigation (Open Type) when you have 100 classes that are all just called "Block". I know it's violating the rule not to repeat the package name in the class, but i'd like to see InterfaceBlock, InterfaceTile (if there are actually both), and so on. In case there's no ambiguity, maybe even drop the suffix.

[yueh]

In this case violating it is probably ok. But dropping the suffix is not an option. Java does not like packages or classes being named Interface, so if it should be consistent with every Block having Block as suffix.

Regarding API: I am fine with the source set approach. The proposals so far didn't mention it so I wanted to make sure it's not getting moved into src/main.

I have only consider it as discussion about the package structure, not the gradle part. That is certainly an option later on. Like moving decorative into its own source set and prevent it from depending on anything but api or core

[shartte]

I don't even think that's necessary, but as you said, we can investigate it later anyway.

[shartte]

I am drafting a PR to investigate how a more explicit registration could look like. Should be done within the day.

[Elix-x]

What do you mean?

[shartte]

Make the registry names explicit.

edit: I also want to consolidate the various base class hacks we currently use to register additional stuff during bootstrapping, like the ItemColor/BlockColor/MeshDefinition stuff. That can all be specified elsewhere.

[Elix-x]

We decided not to do that.

[shartte]

Where?

[Elix-x]

Not sure about that. Sometimes you need a overview of all and that is something current IDEs cannot really do. But the current approach is clearly a bit too heavy magic.

[shartte]

That's not a decision.

[Elix-x]

Well, you can try. We will see how this will turn out.

[yueh]

Being more explicit instead of implictly using the classname is not that bad. But there are some problems with the responsibility. The id is clearly not belonging to an item/block. Otherwise the registry would simply take a specifc interface to determine the id and not pass id + object as separate paramerters. Further it means blocks and items would suddenly have to know about the registries, also something the should not have to.

So currently no idea about a better approach and also not enough time to actually ponder about it.

[shartte]

@yueh I'll make a proposal and yeah, I am not setting the id within the item/block, I also noticed that doesn't make much sense :)

[yueh]

Oh and just to throw it in. Using a static block is probably also a bad idea in general. We have absolutely no idea about the loading order forge or another mod might enforce. So it can easily be in a half loaded state and missing the API to register features.

[Elix-x]

And because we're starting to discuss more changes here, i recently threw in polls in original repo. ~60 people voted and results are following (the poll that was there before, 110 people, had very similar results actually):

• 65%/35% AE3/AE2
• 66%/34% Standart/Modular
• 80%/20% Semantec/Current
• 82%/18% Java8/Java7

So, following these, we can tell that people split almost equally, but would kinda prefer AE3 with standart jar packaging. Meanwhile, there's bigger split towards semantec versioning and java 8 (so our decision of moving to java 8 is totally supported by comunity).

[shartte]

Regarding AE3 vs. AE2: The goal is to first release a version that feature-wise is more or less equivalent to 1.7, right? Even if we rewrite all the internals, a user won't be able to tell the difference I think. So for that version, AE2 still makes sense in my mind.

[Elix-x]

We already decided that (not here), we keep AE2 till we start working on new features.

[shartte]

Hrm, I am struggling with lambdas and @SideOnly. Sadly I run into sided-issues when running it on the dedicated server.

Initially I wanted to do this in the ApiBlocks class for example:

this.skyChestBlock = registry.tile( "sky_chest_block", () -> new BlockSkyChest( SkyChestType.BLOCK ) )
            .features( AEFeature.SkyStoneChests )
            .tesr( SkyChestTESR::new )
            .build();

That won't work ofcourse, since SkyChestTESR is @SideOnly(CLIENT), and that crashes the server. In my second attempt I tried moving all initialization code into a client-only callback:

this.skyChestBlock = registry.tile( "sky_chest_block", () -> new BlockSkyChest( SkyChestType.BLOCK ) )
  .features( AEFeature.SkyStoneChests )
  .rendering( rendering -> {
    rendering.tesr( SkyChestTESR::new )
  } )
  .build();

This actually works on a dedicated server.

When not using a method reference however, the entire thing falls apart and this again doesn't work:

this.skyChestBlock = registry.tile( "sky_chest_block", () -> new BlockSkyChest( SkyChestType.BLOCK ) )
  .features( AEFeature.SkyStoneChests )
  .rendering( rendering -> rendering.tesr( new SkyChestTESR() ) )
  .build();

The entire thing hinges on lambdas being converted automatigically into methods of the outer class. Since it's impossible to annotate them with @SideOnly, this breaks on the server -> :-(

So sadly for now in my proposal, I'll go with this:

this.skyChest = registry.tile( "sky_chest_stone", () -> new BlockSkyChest( SkyChestType.STONE ) )
  .features( AEFeature.SkyStoneChests )
  .rendering( new RenderingCustomization()
    {
      @Override
      public void customize( IRenderingCustomizer rendering )
      {
        rendering.tesr( new SkyChestTESR() );
      }
    } )
  .build();

At least the inner class can also be extracted and placed next to the block. I'll work on this some more and see how nice I can make it. This change will decouple the bootstrapping code that is currently handled via base class methods completely from the logical implementation classes and will allow even Vanilla classes to be used unmodified in some cases.

Simpler cases look like this:

this.grindStone = registry.tile( "grinder", BlockGrinder::new ).features( AEFeature.GrindStone ).build();

[yueh]

I would not even bet @SideOnly working with lambdas at all. That thing is utterly broken.

Also do not forget the server/client proxy.

[shartte]

Yeah it doesn't, you can't place annotations lambdas anyway.

I'll checked out the @SidedProxy, but i think it only works on static fields :|

Well, to be honest it might not be a terrible fix to separate out the rendering customization setup into a class for each block. There are only a handful of those and they can be placed closer to the blocks.

[yueh]

It should be possible to annotate lambds with java8. Just @SideOnly is only designed with java 1.4 or even earlier in mind.

@SidedProxy only exists once. It gets instantiated by forge depending on the side. So you can have a CommonProxy.init(), and let both implement it. Let the client extend server and let it additionally register the rendering part. I am not even sure, if forge wants the tesr in a specific init phase...

[shartte]

@yueh Yeah this code doesn't actually register the TESR right away, it builds a "bootstrap declaration" that then gets split up into the necessary phase initializations. I just want to have all init code in one place so its easy to get an overview.

Regarding @SideOnly: You can't annotate the lambda itself, and if you annotate the functional interface, the annotation is not applied to the synthetic method that is generated for the lambda.

[yueh]

Yeah, I understand the idea behind the builder. Something like initGrindStone() { return super.rendering( r-> r.tesr( new SkyChestTESR())).build() } would be nice. But I currently do not see a nice way for it. That is really something where scala would be an advantage.

I currently have not idea to how lambdas are transformed at bytecode. Probably something of java.util.function and as these are mostly generics, half of forge fails at it. Really do not expect anything newer than java1.4 to be working with forge.

[shartte]

The compiler actually has a complex rule set that governs how the transformation works. It also depends on what's in the lambda and whether it's a method reference or not. In case of the () -> new ... it gets transformed into a standard method in the outer class. And that method ofcourse has the TESR as its return type, which leads to these problems. The anonymous inner class works as a replacement, since you can do the @SideOnly annotation there safely, although it looks ugly. I went with this for now:

this.skyChestBlock = registry.tile( "sky_chest_block", () -> new BlockSkyChest( SkyChestType.BLOCK ) )
  .features( AEFeature.SkyStoneChests )
  .rendering( new SkyChestRenderingCustomization() )
  .build();

And SkyChestRenderingCustomization then lives next to BlockSkyChest with the following content:

public class SkyChestRenderingCustomization extends RenderingCustomization
{
    @SideOnly( Side.CLIENT )
    @Override
    public void customize( IRenderingCustomizer rendering )
    {
        rendering.tesr( new SkyChestTESR() );
    }
}

Currently, IRenderingCustomizer supports block/item colors, TESR and model customization.

Using this approach, I can start the dedicated server and client nicely. Still working through some kinks where I don't know what broke because of elix-x's renaming and what broke because of my changes. Looking into it still.

I pushed my current state to https://github.com/shartte/Applied-Energistics-2/tree/registration but will continue to refine before i submit a PR. The diff is going to be a problem though since the refactoring does touch a lot. The interesting stuff happens in ApiDefinitions and children as well as the classes in appeng.bootstrap

[yueh]

Should be ok. It would also fit when packaging by feature and placing everything related to skychest into the same package.

With scala it would just be class ClientRegistry with TesrRegistry with TileRegistry with BlockRegistry with ItemRegistry and class ServerRegistry with TileRegistry with BlockRegistry with ItemRegistry. Java really misses the advanced features. And the new lambdas, extension methods and similar are just a very weak copy.

[Elix-x]

Branched off 1.10.2-rv3-restructuring specificaly for this.

[Elix-x]

@yueh why does AE has pack.mcmeta? Forge generates default override anyways.

[yueh]

No idea, probably because forge did not always generate it or similar.

[shartte]

Should I kill it?

[Elix-x]

@shartte I will (does not belong to registry refactor, can be done on main branch without damage).

[shartte]

Okay since #75 has been merged, the next thing would be to check all registry names and see if we are satisfied with what they are. After we change any that we want to change, worlds should be safe from being destroyed, even if we decide to move every class around, heh.

I have a few smaller gripes with sky_stone in particular. I know the Wiki calls it "Sky Stone", but wouldn't "Skystone" (one word) be more correct (non-native speaker here, so no idea). And consequently, sky_stone_block_stone should be skystone_block, while sky_stone_block_block should be called something along the lines of polished_skystone, maybe? I'll make a list of suggestions

[shartte]

Suggested Name Changes

Blocks

Old Name	New Name
quartz	quartz_block
quartz_lamp	quartz_vibrant_glass
quartz_torch	quartz_fixture
fluix	fluix_block
sky_stone_block_stone	skystone_block
sky_stone_block_block	smooth_skystone
sky_stone_block_brick	skystone_brick
sky_stone_block_small_brick	skystone_small_brick
sky_chest_stone	skystone_chest
sky_chest_block	smooth_skystone_chest
wireless	wireless_ap
security	security_station
cell_work_bench	cell_workbench
stair_skystone_stone	skystone_stairs
stair_skystone_block	smooth_skystone_stairs
stair_skystone_brick	skystone_brick_stairs
stair_skystone_brick_small	skystone_small_brick_stairs
stair_fluix	fluix_stairs
stair_quartz_certus	quartz_stairs
stair_quartz_certus_chiseled	chiseled_quartz_stairs
stair_quartz_certus_pillar	quartz_pillar_stairs