Add wireless requester terminal support

[Mithi83]

Based on original PR by @Mari023 found here: #11

Open issues

• Right now it adds ae2wtlib as a hard dependency. Mari023's original PR for Fabric had a way to check if ae2wtlib was loaded and only then add the new items. I have no idea how this can be done in Neoforge but it should probably be changed
• I'm not that familiar with the whole maven stuff, this should be double checked.
• It probably needs more cleanup. The main goal for this (and the reason it is a draft PR) is to get some feedback early on.

[rlnt]

On first glance, this looks pretty good. I will investigate further next week and make it an optional dependency. Thanks a lot for contributing!

[Mari023]

Right now it adds ae2wtlib as a hard dependency

the intended way is to JiJ the api mod, that way it'll work with and without ae2wtlib without having to duplicate any code

[62832]

I'd (personally) go even further to say that JiJ-ing isn't necessary, as long as any code involving AE2WTLib is gated behind separate classes which are only referenced and class-loaded conditionally (if the relevant mod is present). This is the approach that e.g. MEGA Cells has used from the very start for all the other add-ons that it provides integration for.

EDIT:

[...] Fabric had a way to check if ae2wtlib was loaded and only then add the new items. I have no idea how this can be done in Neoforge but it should probably be changed

Typically this is done with ModList.get().isModLoaded("[mod_id]").

[Mari023]

I'd (personally) go even further to say that JiJ-ing isn't necessary, as long as any code involving AE2WTLib is gated behind separate classes which are only referenced and class-loaded conditionally (if the relevant mod is present). This is the approach that e.g. MEGA Cells has used from the very start for all the other add-ons that it provides integration for.

that would be an option, but it would mean that you either:

• don't get a wireless terminal when ae2wtlib(_api) is not installed (technically it would still work when some other mod loads it via JiJ, which imo would make it even more confusing for users)
• have to duplicate code for a different (non ae2wtlib) implementation of the wireless terminal

The api mod was specifically made so addons implementing terminals can just JiJ it and don't have to deal with ae2wtlib being maybe present or maybe not

[62832]

Up until now I've already had a general "dummy item" class to use in the event that another add-on wasn't present to provide full integration for. AppliedE, for example, doesn't JiJ AE2WTLib's API either, but it still provides a wireless terminal when the latter isn't installed which simply does nothing as an item.

I assume then that the API also provides its own mechanism for that now?

[rlnt]

I am not a huge fan of JiJ'ing myself personally. Also, not a fan of hard dependencies. It makes maintenance and porting more complicated. If it's a soft dependency, I can just kick it out if it's not ported yet or if it has any issues.

Since the wireless terminal library isn't a dependency that is only available if someone JiJs it, it's not a big deal to make it a soft dependency in my opinion. If people want wireless terminals, they can simply install the mod, and the integration will load. That way, you let the users decide.

Is there something I am missing here? Do you plan to remove the library as a standalone mod? Or do you just suggest JiJ for convenience? @Mari023

[Mari023]

looks mostly good from the ae2wtlib side, some minor changes

you also need a translation for key.ae2.wireless_requester_terminal

[Mari023]

I assume then that the API also provides its own mechanism for that now?

Yes and no.
It can't really do it like you do.
To do that, you'd need to include api code, at which point you might as well use JiJ the whole API, which allows you to register the terminal properly.

I am not a huge fan of JiJ'ing myself personally. Also, not a fan of hard dependencies. It makes maintenance and porting more complicated. If it's a soft dependency, I can just kick it out if it's not ported yet or if it has any issues.

Since the wireless terminal library isn't a dependency that is only available if someone JiJs it, it's not a big deal to make it a soft dependency in my opinion. If people want wireless terminals, they can simply install the mod, and the integration will load. That way, you let the users decide.

Is there something I am missing here? Do you plan to remove the library as a standalone mod? Or do you just suggest JiJ for convenience? @Mari023

Nowadays, there are two parts to ae2wtlib: the main mod ("ae2wtlib") and the api mod ("ae2wtlib_api").
The api mod doesn't really do anything on its own, but it includes everything needed for addons to add terminals with ae2wtlib support without requiring the full mod (in particular it doesn't change ae2 gameplay).

ae2wtlib (and some addons which add their own wireless terminal) JiJ the api mod.

The main mod will always stay a standalone mod.
(the "lib" part in the name was never really true, it is just technical debt that would be annoying to change now)

---

I'd personally recommend to JiJ the api mod, but you can also do it like 90 and and use a dummy item if the api mod isn't present.

[Mithi83]

Hm, I wanted to check those changes and implement the last one by hand but somehow modmaven is not working, getting error code 502 from cloudflare. I'll try again eventually.

[Mithi83]

you also need a translation for key.ae2.wireless_requester_terminal

I don't think I can follow. Why would I need such a translation?

[Mari023]

you also need a translation for key.ae2.wireless_requester_terminal

I don't think I can follow. Why would I need such a translation?

if you don't have that, the keybind will have the raw translation key instead of a proper name
(see the keybind menu under ae2, it should be listed there)

[Mithi83]

Keybind was the bit of information I was missing. Is that added by ae2wtlib automatically?

[Mari023]

Keybind was the bit of information I was missing. Is that added by ae2wtlib automatically?

yes

[rlnt]

Okay, I made a few adjustments, but your base implementation was an excellent foundation. Thanks a lot!

I decided to go the full soft-dependency route and make everything optional. The mod does not ship the lib nor the API. The recipe is now conditional as well, and the item, menu, and screen is only registered if the Wireless Terminal Library mod is installed by the player.

I marked the wtlib mod as an optional dependency with a minimum version range. I think you forgot to test this once again after bumping the version since the new wtlib version actually required a newer AE2 version, which required a newer NeoForge version, but it's all settled now.

The interfaces on the terminal host were unnecessary. The marker interface was an addition by 90 to make the menu work in his full block add-on. The other interface was already implemented by the extended class.

The main crafting recipe had two issues. It had no advancement as an unlock criterion, and the result was using the old item key instead of id.

As a remaining issue, I noticed that menu types were registered twice, but only for the terminal and the wireless terminal, not for the requester itself. This is due to WTLib loading the menu class when passing it to the WTDefinitionBuilder since it doesn't use a supplier. I have now disabled implicit menu registration within AE2 itself for menus from my mod's namespace.

For the future, try to run the mod after all changes have been implemented. Not only to see if all dependencies are there, but also to check for remaining errors or warnings in the logs. I appreciate the contribution. Thanks again! Also, thanks to all the people involved in the discussion.