Incompatibility with Sponge
gabizou opened this issue ยท 23 comments
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 interface
s, 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
.
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?
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
.
Oh right you are talking about MINECRAFT's PropertyHelper. Sorry. That makes it even easier to solve
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?
Yeah, the Minecraft one ;) We've designed a lot of our implementation to specifically handle mods. IUnlistedProperty
and 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.
Would that work? I need to go to bed right now so I'm assuming I've missed something important that will break compat.
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.
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 :)
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.
I mean the 1.8 - 11.14.4.1577 forge version, the recommended version with sponge.
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 :)
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
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.
takes all the time it takes you ; )
for textures , I could recover in next release.
Thank you !
@qwertyazerty Christmas got in the way :)