BuildCraft|Core

BuildCraft|Core

7M Downloads

Incompatibility with Sponge

gabizou opened this issue ยท 23 comments

commented

Currently, SpongeAPI provides an exposed API for working with IProperty instances, but only if those instances are extending PropertyHolder. While granted, our implementation is still a WIP, by not even extending PropertyHolder, we effectively can't cast or utilize/expose the BuildCraftProperty as a BlockTrait.

To clarify: SpongeAPI provides an interface BlockTrait that we partially mixin at runtime with MixinPropertyHelper, but while Mixin has been heavily developed, there are certain cases where we can't mixin to interfaces, such as IProperty, and by extension, BuildCraftProperty.

The workaround fix for compatibility currently until @Mumfrey can make an addition to Mixins in general, would be to properly extend PropertyHelper.

commented

Is this for 1.8.8?

commented

Oh right BuildCraftProperty doesn't exist in 1.7.10. I forgot :P

commented

Also that's not going to happen unless you replace BuildCraftProperty yourselves- I'm not depending on sponge to allow it to work. Sorry.

Out of interest what's the issue with IProperty specifically?

commented

Is this for 1.8.8?

Yes.

Also that's not going to happen unless you replace BuildCraftProperty yourselves- I'm not depending on sponge to allow it to work. Sorry.

We're not replacing BuildCraftProperty, we're simply transforming the class to extend our interface at runtime. If you extend PropertyHelper, it's already compatible, however, if you don't, well, we can't cast.

There in lies the issue that we can't target an interface to implement our own interface. We CAN however target a static class to implement our interface, which is why we have to target PropertyHolder.

commented

Oh right you are talking about MINECRAFT's PropertyHelper. Sorry. That makes it even easier to solve

commented

BuildCraftProperty was created to make generic types easier in 1.8, but now that 1.8.8 exists (with generics) it may no longer be needed and BC can use vanilla's property classes instead :)

This doesn't solve the problem if you want to access BuildCraftExtendedProperty however.

Could this sort of thing be avoided with a seperate mod (Say, BuildCraftCompat) if it were to add the interface to the class?

commented

Yeah, the Minecraft one ;) We've designed a lot of our implementation to specifically handle mods. IUnlistedPropertyand by extension IExtendedBlockState is something we haven't tackled with as of yet, mostly because it's forge specific (we do have plans to add various forge specific things to the API as necessary to interact with mods), but we try to do as much as we can ourselves without requiring mods to do anything to be compatible with SpongeForge.

commented

Would that work? I need to go to bed right now so I'm assuming I've missed something important that will break compat.

commented

Currently, since we don't target IExtendedBlockState, there's little compatibility issue at the moment, however, it is planned to expose to our API. At the moment, you don't need to do anything since the fix from your end is in.

commented

Are you going to compile this for forge 1557 ?

commented

Hello, is this a response to SpongePowered/SpongeForge#457 ? I ask because forge 1557 is an old 1.7.10 version so I assume you didn't mean that :)

commented

One of the differences is that Buildcraft is already targeting 1.8.8 while SpongeForge targets 1.8 until the 1.8.8 builds are no longer in "alpha", so I'm only guessing that he's talking about sort of compatibility.

commented

I mean the 1.8 - 11.14.4.1577 forge version, the recommended version with sponge.

commented

I ask this because i need it on my 1.8 server....

commented

Thanks for pointing that out @gabizou I missed that :)

@qwertyazerty I can re-implement the fix for 1.8 (BC targets 1.8.8 now) and release that with migration features. However because BC no longer targets 1.8 I wouldn't recommend using it as it won't be getting all of the normal bug fixes and features that 1.8.8 has. Hopefully forge will be out of beta shortly so SpongeForge can update to 1.8.8 :)

commented

You can compile it for me if possible 1.8 ?
I need this in 1.8 because all others mods i use don't support 1.8.8

commented

Yes but it will take a little while to do so :)

commented

How long ? I can wait.

commented

I can't guarantee a time (or date unfortunately) because it may not work properly or other issues may arise but it might be done by tomorrow. It might be earlier though, but I don't want to give you false hope.

commented

takes all the time it takes you ; )
for textures , I could recover in next release.
Thank you !

commented

Have you done ?

commented

@qwertyazerty Christmas got in the way :)

commented

Have you done ?