Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Mod-Incompatibility] SpongeForge needs ModMetaData class to remain un-classloaded before CoreMod instantiation #2482

Closed
gabizou opened this issue Oct 17, 2016 · 99 comments

Comments

@gabizou
Copy link

gabizou commented Oct 17, 2016

This is realistically a simple change of how AE2's core mod initializes itself. Because the CoreMod IFMLLoadingPlugin is indirectly classloading classes before other coremod transformers have a chance to transform them, certain integrations may fail outright. An example is available here: SpongePowered/SpongeForge#966.

Now, the change required is rather simple (of which I have written a PR to rectify the incompatibility), simply separating the DummyModContainer out from the IFMLLoadingPlugin. The functionality otherwise does not change the mod or the game in any other way.

gabizou added a commit to gabizou/Applied-Energistics-2 that referenced this issue Oct 17, 2016
…ssary classloading of certain classes. Fixes AppliedEnergistics#2482.

Signed-off-by: Gabriel Harris-Rouquette <gabizou@me.com>
gabizou added a commit to gabizou/Applied-Energistics-2 that referenced this issue Oct 17, 2016
…ssary classloading of certain classes. Fixes AppliedEnergistics#2482.

Signed-off-by: Gabriel Harris-Rouquette <gabizou@me.com>
@yueh
Copy link
Member

yueh commented Oct 17, 2016

I am a bit hesitant about it regardless of how small it might look.

Our stance was always "no support" for anything but forge. For example to avoid something like "but you did support that other bukkit or whatever fork". Especially when anything starts to transform forge classes.
We simply do not have the time to test every release to work with every bukkit fork out there. Which means any release could break any chance of updating these servers to patch a critical issue.

Further as you said it touches the classloading itself. With how broken the forge one already is, I would not be surprised, when this approach is one of the reasons, we did not have many issue in 1.7.10. Thus I currently have no idea what issue this could create for us in the future.

Honestly you could even consider it as design problem of sponge. The JVM already provides a standard way to intercept classloading at any point and allow it to transform the bytecode without depending on the loading order of some random mods. But sponge (and forge) choose to use their own implementation instead and now depends on hoping nobody it outside the intended but unspoken idea.

@Mumfrey
Copy link

Mumfrey commented Oct 17, 2016

I'll just jump in to clarify, since gabi sent me a message to say he's at work right now and can't answer.

I am a bit hesitant about it regardless of how small it might look.

This is perfectly understandable, it's your mod and of course if someone comes along and asks you to change something which - from your point of view - is working perfectly well, then it's obviously going to seem a bit odd, regardless of how well-meaning the other party is. Understanding this, please hear us out here.

Our stance was always "no support" for anything but forge. For example to avoid something like "but you did support that other bukkit or whatever fork". Especially when anything starts to transform forge classes.

Just for the sake of clarity, Sponge is not a bukkit fork or anything like that, it's an entirely new API with some funadmentally different priciples, one of which is direct integration and inter-operation with Forge. Because of this, we're not shoe-horning something which was never designed to be Forge-aware into a Forge environment, we are in fact acting just like any other mod in this instance. SpongeForge itself is actually just a Forge mod which "hosts" the Sponge API.

Now here's the dilemma: Sponge is obviously operating at the same kind of level as Forge or other tweak-level technologies and thus integrates with Forge at a much lower level than most mods - for example in order to provide seamless integration between the two event systems without lots of hugely-unperformant marhsalling, we integrate directly into the Forge event bus. Likewise for plugin loading, we actually plug directly into Forge's mod loading capability rather than "re-inventing the wheel" by doing all the plugin enumeration separately. To all intents and purposes Sponge plugins are just a special kind of Forge mod, the SpongeForge subsystem then does the rest. The problem arises because instead of doing any fancy wrapping of forge, or making custom builds which are just inconvenient for everyone, we actually just leverage the CoreMod functionality already supported by Forge in order to hook into the game.

Essentially this decision was made in order to make everyone's lives easier - in particular modpack authors and end users - who merely need to know that to install Sponge, they simply install the SpongeForge mod like any other mod. Because SpongeForge doesn't get any special treatment from the Forge loader, we have to perform any subsystem-level injections (like hooking into ModContainer and of course the event bus) right at the very first stage of loading - this is by design, since any "special" functionality added to support SpongeForge by Forge could equally be exploited by any other mod and thus would effectively just result in a kind of "special treatment arms-race" as the mods competed for special attention. This is why it might seem like an odd decision to just work as a regular coremod, but ultimately makes the most sense to do so.

This lack of special treatment also leads to some problems though, mainly due to the fact that the CoreMod contract is quite loosely specified. Since Forge just loads coremods in alphabetical order, any coremod which loads before SpongeForge has the capability to disrupt us by causing something crucial to be classloaded before our own coremod class is visited. In some cases it can be identified as being a clear-cut issue with the mod (eg. the mod is causing the event bus to be loaded during coremod init, which is not really allowed) however sometimes it's something where the mod isn't in any way in the wrong, but causes something to be classloaded early, just like in this case.

This is where we try to reach out to mod developers in order to see if they're willing to make a change to their mod in order to allow SpongeForge to work. Wherever possible, we try to make a PR or diff of the changes so that the mod developer isn't actually even caused to have to write any code, which is what gabizou has done in this case. Obviously at this point the developer is perfectly at liberty to tell us to get stuffed, but it doesn't hurt to at least ask the question 😄

We simply do not have the time to test every release to work with every bukkit fork out there. Which means any release could break any chance of updating these servers to patch a critical issue.

This is obviously a given, but I'd urge you to consider this a coremod-coremod compatibility issue rather than a "spurious broken bukkit fork" kind of issue, since we're basically operating at the coremod level with Sponge, and reaching out as one mod developer to another effectively.

Further as you said it touches the classloading itself. With how broken the forge one already is, I would not be surprised, when this approach is one of the reasons, we did not have many issue in 1.7.10. Thus I currently have no idea what issue this could create for us in the future.

Basically, it's a coremod, it just runs transformers just like any other coremod. The difference is that we need (by nature) to actually transform some of the forge classes, and it's at this point that the class load order becomes relevant. If another coremod uses a class before our transformer can touch it, then our mod cannot operate correctly in terms of providing services to the plugins it hosts. As I said above, this isn't implying you're doing anything wrong, but we are asking kindly for a small change to be made - which basically doesn't affect the functioning of your mod at all - in order to accomodate this.

Honestly you could even consider it as design problem of sponge.

As I mentioned above, yes it kind of is a design problem of Sponge, but it was intentionally made that way in order to promote (ironically) compatibility. Basically by being just a regular mod, it greatly simplifies the installation and maintenance of servers for end users. We could potentially work around this by providing some kind of custom launcher which loads us at the same time as Forge, but to make things easier the current method of installation being "just drop it in the mods folder" provides a good experience and means it can be easily removed, upgraded, etc.

The JVM already provides a standard way to intercept classloading at any point and allow it to transform the bytecode without depending on the loading order of some random mods. But sponge (and forge) choose to use their own implementation instead and now depends on hoping nobody it outside the intended but unspoken idea.

You are absolutely correct, however it would again require some kind of custom wrapper in order to launch the game (eg. with a Java Agent, which is what I assume you're referring to). Operating as a coremod gives us the advantages and restrictions of LaunchWrapper, and honestly we aren't in a position to do anything about that if we want to be a Forge-powered product.

In a nutshell, if you tell us to get knotted, that's absolutely fine! However we will always reach out to modders in the community if it's possible to achieve compatibility with relative ease, and I would respectfully ask for your consideration of this issue, since accepting this PR represents the minor changes needed to achieve compatibility. If there's anything else we can offer to demonstrate our best intentions then just let us know. If you're concerned about future support and compatibility then I would like to assure you that we're in this for the long haul, and if we break anything then we take responsibility for it and try to work with other modders to resolve things.

Many thanks for your consideration.

@yueh
Copy link
Member

yueh commented Oct 17, 2016

To clearify it, I use bukkit in terms of the whole approach of providing another API on top of forge while simultaneously heavily modifying forge to achieve it. I am completely aware that sponge is not a real bukkit fork, but more of a spiritual successor to it.

Regarding the class loading. Forge does not even load them in alphabetical order. Their source claims it is using a case insensitive order, but it can clearly load a mod starting with T before another with a. So at least it is case sensitive or just not working. Further it can easily put you into a situation, where it claims to load something from mod A according to the documentation and as this is declared as owner. Which like many other forge systems is actually NYI. So it falls back to mod B and claims it as source, because that was the first jar it discovered for whatever reason. While the classloader is actually loading it from mod C, which is never stated anywhere, except you query the classloader for every single classfile...

Further we want to revisit the coremod itself. Maybe we could even get rid of it. Or if not, it still might magically work afterwards.

But the official statement is always "only forge". Except should forge promote sponge as some officially supported API.
Everything else will always be "at your own risk". It might work now, but we will not ensure that every build will work or will get an immediate fix. Should we ever change it, at least I would personally feel obligated to guarantee it being compatible with whatever we choose to officially support.
Which also opens the door of "you are already support ing X. Why not Y?".

Thus saying no is simply the easier way to not favor a specific thing. Especially as the problems bukkit did face can certainly happen to other projects. To some degree even Forge puts the a similar risk on it currently.

@Mumfrey
Copy link

Mumfrey commented Oct 17, 2016

Regarding the class loading. Forge does not even load them in alphabetical order. Their source claims it is using a case insensitive order, but it can clearly load a mod starting with T before another with a. So at least it is case sensitive or just not working. Further it can easily put you into a situation, where it claims to load something from mod A according to the documentation and as this is declared as owner. Which like many other forge systems is actually NYI. So it falls back to mod B and claims it as source, because that was the first jar it discovered for whatever reason. While the classloader is actually loading it from mod C, which is never stated anywhere, except you query the classloader for every single classfile...

Sorry I wasn't implying that classloading happens in alphabetical order, I was referencing the order that coremods - specifically - are enumerated. Since the enumeration order is the loading order of the coremod class itself (eg. the class implementing IFMLLoadingPlugin) it is this which determines the order in which coremods get to do their init things. Thus we are not actually classload order dependent, but just sensitive to things being loaded at this very early stage of pre-initialisation. Essentially, it's only this pre-init phase where things are problematic to us, not at any point thereafter, the rest of the software stack is extremely adaptible.

Further we want to revisit the coremod itself. Maybe we could even get rid of it. Or if not, it still might magically work afterwards.

That would certainly solve our issue too in this instance, though in the short term the changes proposed by gabizou actually will fix it without changing any of your mod's functionality.

But the official statement is always "only forge". Except should forge promote sponge as some officially supported API.

Well Sponge is a Forge mod and we work closely with Forge to develop this system, they added support for us to hook into the mod loading very early on and they host our downloads and SpongeForge itself builds on the Forge Jenkins, so I don't know how much closer we could get without them writing our code for us lol! As I said, the reason for the mod itself operating at the coremod level is to promote compatibility and ease of installation above all else.

Everything else will always be "at your own risk". It might work now, but we will not ensure that every build will work or will get an immediate fix. Should we ever change it, at least I would personally feel obligated to guarantee it being compatible with whatever we choose to officially support.

I'm not sure I follow. I mean this is essentially an intra-mod compatibility measure, and literally the only change is to move something which doesn't actually need to occur in the PREINIT stage into a later stage, in order to avoid the dummy modcontainer being loaded too early for us to connect with it. It does not in any way represent an endorsement of our mod on your part, it merely creates compatibility in a straightforward way, without any collateral effects of any kind. So it's not really a "support" thing, merely a mod compatibility thing.

Thus saying no is simply the easier way to not favor a specific thing.

I appreciate that saying no is of course the easiest way, however accepting this PR doesn't really endorse our mod or anything like that. It simply allows coremods to be non-interfering by moving an initialisation step slightly later in the game's startup process.

Especially as the problems bukkit did face can certainly happen to other projects. To some degree even Forge puts the a similar risk on it currently.

Sorry I've completely lost you here. SpongeForge is a Forge mod first and foremost. We feel that using Forge represents the absolute best opportunity for compatibility, and the approaches and entire ecosystem of bukkit don't really apply. We're working with and as part of the modding community first and foremost, that's why we try to resolve these kind of issues by reaching out to other modders in the community when possible rather than being seen to be separate.

Basically we are asking, politely, as one mod developer to another, if you would consider changing this behaviour for compatibility purposes. Please don't think that we are asking for you to endorse us or anything like that, we wouldn't be so presumptious. The PR is - as I mentioned above - simply to try and make it as easy as possible for you, so that we're not putting extra work on your plate as it were.

@yueh
Copy link
Member

yueh commented Oct 17, 2016

Sorry I wasn't implying that classloading happens in alphabetical order, I was referencing the order that coremods - specifically - are enumerated. Since the enumeration order is the loading order of the coremod class itself (eg. the class implementing IFMLLoadingPlugin) it is this which determines the order in which coremods get to do their init things. Thus we are not actually classload order dependent, but just sensitive to things being loaded at this very early stage of pre-initialisation. Essentially, it's only this pre-init phase where things are problematic to us, not at any point thereafter, the rest of the software stack is extremely adaptible.

Yes, I am aware of it and meant the order on how core mods are loaded and how it then affects the later class loading. So for example should some idiot ship outdated parts of the SpongeAPI, then forge can prefer to load these over the official ones.

Well Sponge is a Forge mod and we work closely with Forge to develop this system, they added support for us to hook into the mod loading very early on and they host our downloads and SpongeForge itself builds on the Forge Jenkins, so I don't know how much closer we could get without them writing our code for us lol! As I said, the reason for the mod itself operating at the coremod level is to promote compatibility and ease of installation above all else.

Something along the line of shipping Sponge as part of Forge or having an official endorsement in terms of "look here for a plugin api" in in somewhat important places. Forge for example also hosts AE2 downloads, but I would never consider it an official forge mod due to it.

intra-mod compatibility

This goes a bit deeper as just clashing core mods. For example we cannot use the normal block events provided by forge for parts. So should someone make a anti griefing plugin for Sponge, this will never protect any AE2 parts. Say someone wants to make something like a public storage and wants to only allow access to the storage itself. As the block interaction event is not sufficient, it is not possible to allow viewing the terminal without simultaneously allowing anyone to diassemble the whole system, which is the same at block level. To support it either Sponge has to provide custom events for these use cases and we have to actively support Sponge and not just Forge. Or Sponge has to support custom AE2 events, which is also stupid once Sponge (or the protection plugin) has to support hundreds of different mods and their custom events. While any mod not doing it will still circumvent any protection provided by Sponge.

Also for example the current issue is related to the ModMetadata and not the container, which for example also provides a constructor to inject the md from externally instead of constructing our own.
Spontaneously I have no idea, if it for example could not fix. Without having a useless DummyModContainer just to show the version number of an empty mod or similar.

It is simply not a high priority to actually consider it right now.
We have to deal with so many other important things first. Adding it now means people will use it in Sponge servers, report bugs which might just vanish once we stablize the alpha for forge. Which means we just waste time to triage bugs instead of fixing them.

Sorry I've completely lost you here. SpongeForge is a Forge mod first and foremost. We feel that using Forge represents the absolute best opportunity for compatibility, and the approaches and entire ecosystem of bukkit don't really apply. We're working with and as part of the modding community first and foremost, that's why we try to resolve these kind of issues by reaching out to other modders in the community when possible rather than being seen to be separate.

Forge has currently the problem of breaking copyright laws in a few countries. Which can potentially put it at risk. I have not looked at Sponge, but if there is a chance for someone being able to execute an strategy of scorched earth like with bukkit, I am a bit hesitant currently.

@Zidane
Copy link

Zidane commented Oct 17, 2016

This goes a bit deeper as just clashing core mods. For example we cannot use the normal block events provided by forge for parts. So should someone make a anti griefing plugin for Sponge, this will never protect any AE2 parts. Say someone wants to make something like a public storage and wants to only allow access to the storage itself. As the block interaction event is not sufficient, it is not possible to allow viewing the terminal without simultaneously allowing anyone to diassemble the whole system, which is the same at block level. To support it either Sponge has to provide custom events for these use cases and we have to actively support Sponge and not just Forge. Or Sponge has to support custom AE2 events, which is also stupid once Sponge (or the protection plugin) has to support hundreds of different mods and their custom events. While any mod not doing it will still circumvent any protection provided by Sponge.

Erm, this doesn't affect Sponge. Sponge's tracking system tracks at the deepest levels and do NOT need special support on your end or our end. If someone cancels our basic event of placing a block that happens via your mod, it will stop your mod. This is one of the biggest reasons why people use Sponge: it allows them to protect their server without forcing a mod developer to fire events. For example, Thaumcraft fires almost no events for its magic it performs yet Sponge blocks it.

Forge has currently the problem of breaking copyright laws in a few countries. Which can potentially put it at risk. I have not looked at Sponge, but if there is a chance for someone being able to execute an strategy of scorched earth like with bukkit, I am a bit hesitant currently.

Sponge is MIT licensed, there i no issue with breaking of copyright laws.

@yueh
Copy link
Member

yueh commented Oct 17, 2016

If someone cancels our basic event of placing a block that happens via your mod, it will stop your mod

The problem here is there is no block placing event at all. It has to completely bypass it because removing or placing a part is essentially the same as opening a chest or some GUI. So allowing to just view the chest content, will also allow the modification of parts.

In case of a block break, that is mostly just a cleanup operation, once there is no part left in the same blockspace. Preventing this can easily cause that we drop the actual part, while whatever plugin prevents the block/tile from being changed at all. So suddenly you have nice way to dupe enabled by the which might be intended to prevent it.

Sponge is MIT licensed, there i no issue with breaking of copyright laws.

MIT has the problem of not considering contributions. So someone could contribute code and then claiming 6 months later they never contributed it as MIT and request it to be removed.
Not saying that MIT is bad choice, just like any other it has certain downsides.

@Aaron1011
Copy link

Aaron1011 commented Oct 18, 2016

MIT has the problem of not considering contributions. So someone could contribute code and then claiming 6 months later they never contributed it as MIT and request it to be removed.

But, Forge has exactly the same issue. While they do use a CLA for patches, non-patch code does not have its copyright assigned to Forge. If that's what you're worried about, Sponge really isn't at any more risk than Forge is.

The problem here is there is no block placing event at all. It has to completely bypass it because removing or placing a part is essentially the same as opening a chest or some GUI. So allowing to just view the chest content, will also allow the modification of parts.

As @Zidane said, Sponge modifies the base game at a very deep level (replacing setBlockState and spawnEntityInWorld for example), so that it can fire things like block and inventory modification events without plugins ever needing to do anything.

Preventing this can easily cause that we drop the actual part, while whatever plugin prevents the block/tile from being changed at all.

Sponge also allows plugins to detect and cancel the spawning of items that occurs as the direct result of a block or tileentity action - again, without the need for mods to even be aware of Sponge's existence.

I completely understand your concerns about not wanting to have to support arbitrary changes made by other mods. However, as @Mumfrey outlined, the only reason we're requesting this change at all is because of the inherent limitations of LaunchWrapper as it stands now. This is the only way of fixing it without either requiring a special Sponge launcher, or making changes to LaunchWrapper itself.

For all other mod compatibility issues, we strive to ensure that if a mod works with plain Forge, it will work with Sponges. Things like block protection shouldn't require any changes on your part.

@yueh
Copy link
Member

yueh commented Oct 18, 2016

But, Forge has exactly the same issue. While they do use a CLA for patches, non-patch code does not have its copyright assigned to Forge. If that's what you're worried about, Sponge really isn't at any more risk than Forge is.

This CLA is exactly what breaks copyright law. They could just have used a normal CLA without transfering the ownership, but for whatever reason they choose not to. And this is exaclty the issue as transfering the ownership is in some cases either impossible at all or placed under heavy restrictions. Like requiring a written contract between both parties and ideally notarially certified by some official authority to avoid any doubt about contract being forged.

I completely understand your concerns about not wanting to have to support arbitrary changes made by other mods. However, as @Mumfrey outlined, the only reason we're requesting this change at all is because of the inherent limitations of LaunchWrapper as it stands now. This is the only way of fixing it without either requiring a special Sponge launcher, or making changes to LaunchWrapper itself.

The main problem now is that we are still in alpha and have to resolve potential bugs. Adding sponge now to the mix of potential causes is simply a bad decision, especially as it heavily modifies forge. So we have to waste additional time triage these bugs. Are they caused by us? Or Sponge? Or something in between, because we use Forge in a slightly weird way which is ok with forge, but appears once someone uses sponge, etc.

The question is really not about should we fix it or not. But when. While "now!" is certainly less tempting than "sometime during the alpha/beta stage". Like once we had the time to revisit the coremod for our needs. Which might make the DummyModContainer even obsolete.

I see the reason to resolve core mod clashes in any way, because that might even affect forge itself at some point. But anything else is simply out of scope currently.

Nevertheless we will never officially support sponge in terms of plugin usage. So we will always require a bug to be reproduced without sponge. Especially when considering things like sponge mixins. We simply cannot support everyone injecting random code into AE2 and expect it to work. Especially once we start to refactor internal classes and the class/method signatures might change and breaking them.

@Dockter
Copy link

Dockter commented Oct 18, 2016

It is of my opinion that you are severely limiting your player base and and shackling your server administrators by choosing this stance. As a long time server administrator who has used AE2 in the past , we won't be doing it again any time soon due to this. Having a stable server which can be protected and optimized is more important than having features that are contains within someones mod who's decided its not important to work with the rest of the server community to promote compatibility.

@bookerthegeek
Copy link
Collaborator

bookerthegeek commented Oct 18, 2016

While I understand the reasons for both sides of the issue, I am curious as to something. What does this change do? From what I can see (granted I know no Java) this is basically compatability between to mods that both require forge, and in the simplest terms just needs the order things are done in changed.

As this does not say "we support sponge" but more so "compatability between things that run on forge" I fail to see the "why" you are against this.

Just wanting to know what I am missing.

Edit
Also as a server admin, I would like to say that while we won't not run packs that include mods that have issues with sponge, it will influence or decision. Not to be threatening, but if the choice is between a pack with mods that work with sponge and a pack that includes mods that don't, it is an easy choice as to which pack we will be hosting and which pack will not be hosted. Just food for thought.

@yueh
Copy link
Member

yueh commented Oct 18, 2016

@bookerthegeek One issue of me hesitating to support anything bukkit or sponge related is that some random server admins, running P2W servers and violating the minecraft EULA, will show up and start to threating us about losing millions of players as well as hundreds or even thousands of server admins running P2W servers.

This happens absolutely every time something Bukkit related appears. And to be honest losing these types of servers is certainly nothing I would consider a loss. We have even taken down some of these servers in the past.

Further I never said, that we will reject adding at least the basic compatibility regarding the coremods. It just has a low priority. Just be patient.

And face it, nobody with a bit of sanity wants to run an alpha build on a server. Why even make a fuss about it being not fixed immediately, but we say we will look into it before a stable release.
But no these guys always have to come out of the woodwork because they might have realised a potential loss of money due to being unable to provide some specific mods, not having the option to appear as early adopter, or whatever else.

@Aaron1011
Copy link

@yueh: In what way were/could server owners threaten you? You have absolutely no responsibility for people who choose to run your mod on a server which happens to be breaking the EULA.

@aikar
Copy link

aikar commented Oct 19, 2016

FYI You could create a P2W server and break the EULA on Forge too.

@leagris
Copy link

leagris commented Oct 19, 2016

I ran multiple community servers and came to a simple conclusion.
When you need to rely on area/block protection to avoid abuses, your community is socially broken to begin with. This is especially a critical concern on a modded server.
If your player-base is not mature enough to follow common-sense rules to respect other players and the person paying the server bills and dealing with shared resources, without having technically enforced limitations, I doubt it will do any good with complex mods like AppliedEnergistics.
The more technical restrictions you will enforce, the more those immature players will seek a breach, abuse any dupe glitch left behind, bring your TPS to a stop by deploying insane designs...
The other basic community features are already properly handled by more modest mods like server-tools or if you really want, ForgeEssentials.

@yueh
Copy link
Member

yueh commented Oct 19, 2016

@Aaron1011 In no way. But for whatever reason they try it. It just gets annoying at some point. Once I will do some research on them. Which might end up in simply saying screw sponge for openly supporting P2W servers or at least no clear communication of condemning P2W servers/EULA violations.

@aikar While this is certainly possible, I have never seen one. Mostly due things like vanilla and forge always having documented dupe bugs and way less options to prevent it, which certainly will kill any "economy". But the good part is that we do not have to wait for Mojang to do anything about it. P2W is automatically a license violation for AE2, so we can take care of it and take it down by ourselves.


Edit
@leagris Thanks, that is pretty much exactly my opinion towards community servers.

@Aaron1011
Copy link

Which might end up in simply saying screw sponge for openly supporting P2W servers or at least no clear communication of condemning P2W servers/EULA violations.

I really have no idea what you're talking about. Is there a specific example you're thinking of where Sponge supported a pay-to-win server? Keep in mind that our forum rules explicitly disallow plugins which violate the EULA.

@aikar
Copy link

aikar commented Oct 19, 2016

@yueh On terms of "Priority", note a PR has been made for this, and the change itself is simply a matter of "Organization". It's not a fundamental behavior change, just organization.

So there's no chance it will "cause problems" now or in the future. All is needed is a merge and you're good.

Plus, why does environment matter then? If they violate your license on Sponge too, then perform your same "take down" as you would Forge (which note -- they will be using Forge!).
You're not supporting a different platform. You're resolving a mod conflict with another users mod.

Now, not "Supporting" Sponge is a perfectly fine choice, but if you least do this change, it puts it in a "if it works, then that's good for you, but we wont put time into fixing it ourselves" state which is much nicer to your users.

From my understanding, the "issue" present could cause conflicts with other forge mods too beyond Sponge.

@Aaron1011
Copy link

@yueh: One other thing - you said earlier that "We simply cannot support everyone injecting random code into AE2 and expect it to work."

Sponge does not (and probably never will) directly modify mod classes. We do modify many core Forge classes, but we always try to ensure that our changes preserve the original Forge behavior for any plugins using them. This issue isn't specific to AE2 - it's about mods classloading certain core classes very early, before Sponge even has a chance to do anything at all.

For all other types of issues, we can usually either fix the offending code in Sponge, or at least allow Sponge to be configured for mods which need certain workaround (as we did in an issue with ExtraUtils2's redstone clock block). None of this ever requires us to modify the mod code itself at runtime.

@Dockter
Copy link

Dockter commented Oct 19, 2016

@leagris clearly you've never ran a server with more than 5 people. Having ran servers that have 50+ people on all day long whether someone is honest or not you will always find someone who decides to go off the reservation. Having one person ruin it for 49 others by saying "Well if we can all just get along everything will be fine". You need to inject some common sense into this equation. Every server with any substantial size and or length of time online uses something like GriefPrevention, Residence etc etc. Always.

Forge even with ForgeEssentials simply lacks the event system to catch stuff like this. Hell even bukkit's event system was only about 80% capable of catching things. Sponge has proved that their event bus is not only capable but expandable to the point where it can catch and stop even the most ill-coded eventless mods such as Thaumcraft.

@leagris
Copy link

leagris commented Oct 19, 2016

@Aaron1011 @aikar As @yueh said. The fate of the core part of AE has not been decided. The pullrequest has not been denied. Reviewing it now or pulling it now does not make sense. There is no public build of AE2 for 1.10.2 and at least for the development stage, the few guys involved into its development want to focus on a single know reference platform that is Forge. There are enough of blocking issues to deal with as-is.
Your PR or another mean of preserving maximum compatibility has been clearly confirmed to be a consideration of the AE2 developers. It is just not the time for it and it. And sure it will not be an effort toward a specific mod. I trust the current team to do things right when it is time. It is not like @yueh is a newcomer to mod development either.

@Aaron1011
Copy link

@leagris: Sorry, I didn't mean to imply that I was impatient for the PR to be merged - I just wanted to explain why I think it's a reasonable change to make.

@liach
Copy link

liach commented Oct 19, 2016

And fe's code is somewhat sick. Anyways this is the decision of AE dev whether to lose some potential users or to do some code changes. That is all.

@Dockter
Copy link

Dockter commented Oct 19, 2016

As this PR sits though any potential developer wanting to help solve AE2 bugs when running SpongeForge server core can't when we are building our 1.10.x server configuration. I am a prime example of that. If we can't get it to load we can't create tickets or PR fixes. Just my 2 cents.

Help us, help you.

@dotModded
Copy link

I'm sorry but I can't begin to understand your stance on this, I get the not wanting to support sponge. I get all that, but this doesn't just fix sponge it would fix any mod that needs to run the same thing first. I personally am just trying to run a server for friends and want the optimizations that come along with sponge and I wanted AE2however I have to choose sponge over this due to performance constraints. I'm actually disappointed at this because AE2 is such a good mod.

@yueh
Copy link
Member

yueh commented Oct 19, 2016

@mcsnetworks Let us make it easy for you. You are running a P2W server. The Minecraft EULA violation part is none of my business. However it also affects AE2. So you do not even have to bother AE2 not starting, because your are simply not allowed to use it.

At all related to sponge. I am now facing a dilemma. While you claim to to not allow plugins breaking the EULA, you seem to absolutely not concerns about letting members of your organisation run servers which violate the minecraft EULA or participating in more than just supporting some random guy to solve their bugs, but are actively involved with these servers.

My original plan was to look at it today or tomorrow as I already said we have to look at the coremod for our own needs. The sole reason for delaying to press the merge button, was that I am simply not aware of this specific solution being the best one. There are certainly others like simply moving the metadata to mcmod.info and let it inject. Or reuse the injected metadata, when possible. Or whatever else option might be possible. None of these were discussed in terms of the issue or even just a comment about why the proposed one would the best compared to other alternatives. It always sounded more like "We need it. ASAP."

So for now I simply put it on hold until these concerns are resolved.

@dotModded I am actually sorry about that. Of course I cannot speak about the performance improvements. But Mojang themselves often stay away from them because some of these are actually the reason for some of the features/behaviours. Changing it would literally break these. However you can thank the ones running P2W servers for our stance about bukkit or similar projects.

@yueh yueh closed this as completed Oct 19, 2016
@Aaron1011
Copy link

@popcorn9499
Copy link

That worked really well. Thanks for this. The server seems to be running well and it loaded fine and everything. Thank you

@phit
Copy link

phit commented Apr 25, 2017

So is this ever gonna get fixed officially @yueh ? Or are you just planning on closing all issues related to this forever? I mean I don't mind maintaining a fork, but it seems rather pointless when this could be easily fixed here.

@bookerthegeek
Copy link
Collaborator

@markhughes
Copy link

markhughes commented Apr 26, 2017

I thought this was fixed in feature-coremod-refactoring?

See #2636

@gabizou
Copy link
Author

gabizou commented Apr 26, 2017

It is fixed there on sponge side.

It's not fixed on the sponge side.... That's a workaround that isn't possible to use on clients, only for servers.

I thought this was fixed in feature-coremod-refactoring?

Nope, this is 100% to do with the fact that the core mod itself is still instantiating and class loading classes it shouldn't, refer to the changes I made in the PR that was not accepted as previously discussed: #2483

@markhughes
Copy link

Oh this looks like we're just going around in circles. 🙄

@yueh I think it may be best to consider the fix created by @gabizou, or have a call for maintainers. Gabriel is a trusted developer in multiple MC development communities. I would vouch, and I'm sure many other people would vouch.

If it's never going to be supported because you don't like Sponge say it so we can all move on.. 🏳️

@bookerthegeek
Copy link
Collaborator

@gabizou how is this not able to be used client side? The issue is with sponge and AE. Without sponge then no issue... With sponge you can include the fix. What am i missing?

@LemADEC
Copy link

LemADEC commented Apr 26, 2017

When investigating issues, you want to have sponge in SSP. There's also plugins support on client side, not sure how wide it's used in practice.

@simon816
Copy link

@bookerthegeek SpongeBootstrap patches Forge, it does not "fix" the incorrect code in AE2 nor does it "fix" anything in SpongeForge. It patches Forge to load SpongeForge's coremod before any coremods get loaded from the mods/ directory.
Also, I have a note on the SpongeBoostrap post:

Using this should be a last resort if there is no viable alternative.

The patch provided by @gabizou correctly fixes the issue. A fork of AE2 with the patch can be found here: https://github.com/Stonebound/Applied-Energistics-2
This is the correct fix and does not require any modifications to SpongeForge or changes to the way the server gets loaded.

I'm sure it's been explained already, but just to reiterate, AE2's AppEngCore (the coremod) is loading DummyModContainer.
Now, coremods must be loaded in the root classloader (AppClassLoader).
All normal mod code (as well as Minecraft code) must go through the LaunchClassLoader so that coremods get a chance to apply bytecode transformations.
DummyModContainer is a non-coremod component and therefore should be classloaded through LaunchClassLoader.
We can see the problem here, AppEngCore is forcing DummyModContainer to be loaded through AppClassLoader.

You may say "This isn't an issue with AE2, look, it doesn't happen with just Forge"

Just because Forge doesn't give an error it doesn't mean the mod is behaving in a mod compatible way.
At this moment in time, Forge does not issue a warning when classes are loaded in the wrong class loader. However, this is a valid error condition because only coremod classes (and some core Forge classes) should be loaded directly with AppClassLoader.
This issue is not related to SpongeForge, no amount of changes to sponge's code would stop DummyModContainer getting loaded with AppClassLoader when AE2 is present.
SpongeForge has only highlighted this issue because it made the assumption that DummyModContainer was loaded with LaunchClassLoader.

I'm not sure what else can be said on the issue, remember that it's not SpongeForge at fault here.

Don't shoot the messenger.

@bookerthegeek
Copy link
Collaborator

@simon816 thank you for the explanation.

@3TUSK
Copy link

3TUSK commented Sep 29, 2017

For people who are not aware about recent updates, in commit c4fa21c (included in rv5-alpha build 3 and later version), the coremod used in AE2 is removed and thus this issue is "officially addressed" to a certain extent.

@p455w0rd
Copy link

p455w0rd commented Sep 29, 2017

gotta say..this issue has given me newfound respect for @yueh ...yueh is human and do agree with most of what yueh has decided to do here...it's logical and @simon816 has also provided the best most in-depth explanation possible...imo..the best solution would be to implement whatever 'plugins' are needed via a mod and Forge's API...this is totally possible..just hasn't been done yet to the capacity of plugins like GreifPrevention...let this issue die already...there ARE solutions outside of both AE2 and Sponge/SpongeForge...someone just has to do it..

@yueh
Copy link
Member

yueh commented Sep 29, 2017

I would not say @simon816 is the best explanation. It is just a detailed overview about what is happening, which was never questioned, as well as explaining how sponge might not be the bad guy.

But both sides are just guessing about the intention of forge. With forge making it more and more tedious to use coremods, it is perfectly fine to argue that coremods themselves are bad thing and should never have existed. They are just a necessary evil for now. Under that assumption, it is easy to argue that AE2 is just the messenger telling sponge to fix their shit, because patching forge might never be intended at all. It was just too hard to protect against it, so it simply was ignored. Therefore sponge is just exploiting it and claiming it is mod compatible, because forge itself does not crash should they attempt to patch core classes. Thus it is not mod compatible itself.

Not saying that either case is the right one. It is just not our call to blame who is wrong here. That is up to forge and as long as the code and/or documentation does not state what to do and what not, neither is correct or incorrect. For now it is just a dispute about interpreting forge. Nothing more, nothing less. Also it is not the only case, in which AE2 and sponge have clashes due to sponge putting more constraints on their own interpretation.

IMO core mods themselves should be completely abandoned for various reasons. Not just to remove potential problems due to incompatible patches etc. It is also a huge security issue. Lex himself claims that he never wants to add an auto download functionality for mods to fetch missing libs. As they could execute arbitrary and malicious code. But he is perfectly ok with a mod patching forge to add this functionality. In both cases it is still the same attack vector. Someone places a random .jar into forge and have it executed. The only difference is that forge would accept an URL/URI and handle the download, while currently the mod itself would have to download it. It is literally a fig leaf. And not even thinking that the whole point of forge is to execute code from dubious sources. So even without coremods, it is still the same issue.

But getting rid of core mods would mean, that forge would have to be more open to accept patches for more detailed hooks as well actually be more constraint about what is intended. So more docs as well as keeping them updated. And not just the "read the code and interpret it for yourself". But that does not seem like it was ever a high priority and if docs actually exist, they are often outdated or even contradictory with forge providing a more better method, but the deprecated vanilla method still exists and has to be implemented as the patches for the new one are incomplete..

@asiekierka
Copy link

Also, there are many things for which not having a coremod is outright impossible, and I doubt modders creating them would take "no" for an answer - some of them already made their own mod loaders just to pursue the vision. The clearest example is probably OpenCubicChunks (originally cuchaz's Cubic Chunks for cuchaz's own M3L loader).

@mindforger
Copy link
Collaborator

mindforger commented Sep 29, 2017

It is literally a fig leaf

but it's a big one in terms of responsebility ... if i leave my class environment open to overwrite parts with code that can harm people, it is primarily not my fault or carelessness at most
but if i add a feature that gets abused and i can not show that i tried everything possible to avoid an abuse ... i am the one who is responsible for the daamage in the end

i am speaking from a standpoint where i am developing firmwares for electronics, so i can exactly tell where to draw the line of responsibility in this case ... and any feature that allows to do funny stuff has to be either abandoned or it has to be "air thight" or some customer will eventually blame us for damage that has been done with this feature

as an example, you share the codebase with some branches that are available to download in public. ... we even had an issue reported that was based on one of these branches
but you deny any support for those .... but if it would be an addon that is using your API and there is a bug that arises using that API you would have to support them at least if they can prove your API is doing somethign bad

@yueh
Copy link
Member

yueh commented Sep 29, 2017

Also, there are many things for which not having a coremod is outright impossible, ...

Which most likely leads to forge should be more open for new hooks/etc (also AT could still exist). So coremods are not needed. But it should also be about mod development and loading. Not patching minecraft. That should be handled at a completely different level. So finally it is just a compatibility issue between different mod loaders and not offload it to hundreds or thousands of mods and leave them to solve every potential permutation of them.

but it's a big one in terms of responsebility

Maybe. But in case of forge, it executes every piece of untrusted code you throw at it without any sandbox/etc.

i am speaking from a standpoint where i am developing firmwares for electronics,

I doubt any customer would accept an excuse about "Look, we only provided a full blown scripting engine with root access and a TCP stack as part of the firmware, but we intentially did not add a HTTP lib to allow someone from executing a script to fetch further code from pastebin. Not our issue that some copy&pasted a malicious script with HTTP support into it.". There is no difference between the script having to implement their own HTTP lib or not. The whole point of forge is to do funny things and be not airtight to make the funny things possible.

Did not some mods in 1.7.10 or 1.6.4 download their libs, iirc some codechicken?

as an example, you share the codebase with some branches that are available to download in public

We usually do not provide support for old branches. Because how can we provide support, when users are not willing to update? And no project will support something after EOL. Or custom builds, as there is no way to tell what they have changed. (One reason I would like to use signed .jars)

@mindforger
Copy link
Collaborator

We usually do not provide support for old branches. Because how can we provide support, when users are not willing to update? And no project will support something after EOL. Or custom builds, as there is no way to tell what they have changed. (One reason I would like to use signed .jars)

i am not talking about "old" branches, Applied Llamagistics is for 1.12 and there had been an issue that i just can not find anymore that has been posted over here ... i thought this would be an perfect example :)

but this is going offtopic now ... and in regards to the other "abusable" stuff you are right ... i was just under the impression from what i have read about that, it really was a concern about security and responsibility

@asiekierka
Copy link

Which most likely leads to forge should be more open for new hooks/etc (also AT could still exist). So coremods are not needed.

No, yueh. If you add every hook needed for every coremod ever, you'll just slow down and bloat up the game massively for the 95% of players who will never use a single one of them.

Not everyone needs a render engine patched up to support shaders. Not everyone needs a Chunk class capable of handling infinite heights.

@Mumfrey
Copy link

Mumfrey commented Sep 29, 2017

Sorry to add more to this, but honestly this has just gone into the realms of speculation again when I made a lengthy post nearly a year ago explaining exactly what happened.

@yueh said:
But both sides are just guessing about the intention of forge.

No, we are not. The developers of Sponge were working with the developers of Forge from day (-2) in the nextstep chat, and by day zero we had thrashed out the strategy for development, in that we would not use binpatches or distribute a customised forge because it would be easier for long-term maintainability and compatibility to distribute the package as a runtime dynamic patcher. It was Lex himself who literally told me to collaborate with Zidane to repackage the tech I had in LiteLoader into a patching core that Sponge could use.

As I already said a year ago, Forge could have added some "subsytem hook" for us, but the coremod hook had plenty of power and any hook added for us could just be used by other mods anyway, and would defeat the object, it would just move the problem along rather than making it go away.

With forge making it more and more tedious to use coremods, it is perfectly fine to argue that coremods themselves are bad thing and should never have existed. They are just a necessary evil for now.

Well yes, but the actual problem with coremods is not the coremods themselves, but modders acting irresponsibly in using them. I have gone so far out of my way over the last 3 years to ensure that Mixin is as interoperable and compatible as possible, it's one of the core tenets of its existence. Yes coremods can be bad, but ultimately the problems come out of people doing dumb shit with them.

Under that assumption, it is easy to argue that AE2 is just the messenger telling sponge to fix their shit, because patching forge might never be intended at all.

Again, speculation on your part which is actually inaccurate. You aren't "telling Sponge to fix their shit", quite the reverse: we are telling you (have told you) that your coremod takes an irresponsible and unsupported action which you get away with because other agents are not sensitive to the side-effects of your actions. Sponge is - in good faith - using an established hook to provide a level of functionality orthogonal to Forge itself, without breaking anyone's stuff, but with the trade-off of being sensitive to coremodders who decide to act irresponsibly.

Let's not forget also that when this issue is discovered, Sponge developers will take the time to rectify the offending code - quite often because it's just an unintended and accidental side-effect of a coremodder doing something they didn't actually realise was incorrect, and so was done with no malintent whatsoever - and then submit a pull-request to address the issue so that the coremodder themselves is not burdened with having to make changes that do not directly benefit them. Much like gabizou did in this case, he provided you with changes up front which did not impinge on functionality but would simply rectify the issue as it stood.

It was just too hard to protect against it, so it simply was ignored.

Again, contradicted by the facts in hand, which is that we considered lots of ways of hooking into forge in conference with the developers of Forge, and this was chosen as the most reliable.

Therefore sponge is just exploiting it and claiming it is mod compatible, because forge itself does not crash should they attempt to patch core classes.

Again, you're utterly missing the point. Sponge must hook into those classes, this is the best way, it works reliably when coremodders stay within their remit.

Thus it is not mod compatible itself.

Since the whole point of Sponge is to be mod compatible by hooking directly into forge at a low level this is just patently false.

Not saying that either case is the right one. It is just not our call to blame who is wrong here.

We gave you a simple fix. You are too stubborn to use it. Who's in the wrong exactly?

That is up to forge and as long as the code and/or documentation does not state what to do and what not, neither is correct or incorrect.

We work with the developers of Forge, we literally sit in each others private irc channels and discuss this shit all the time. Regardless of whether it's documented we are confident in our assertions.

For now it is just a dispute about interpreting forge.

There is no dispute, there is just you, sabotaging your own user base over an extremely petty prejudice.

IMO core mods themselves should be completely abandoned for various reasons.

Lex and cpw would agree with you, but from a pragmatic point of view it's never gonna happen.

Not just to remove potential problems due to incompatible patches etc. It is also a huge security issue. Lex himself claims that he never wants to add an auto download functionality for mods to fetch missing libs. As they could execute arbitrary and malicious code. But he is perfectly ok with a mod patching forge to add this functionality. In both cases it is still the same attack vector.

False equivalence, once again you're taking someone else's standpoint and ascribing your own (incorrect) interpretation of it. It's not the same attack vector at all. The point is not about defending the users system from the actions of mods, that's impossible. The point is about defending the users system against things they havent chosen to install. And that choice is transitive.

If you install forge. You have chosen to install forge. Forge will never choose to install other agents without your consent. Now if you install Mod A, and Mod A takes some undesired action (such as formatting your hard drive, or downloading Mod B with does some nefarious action) then it's still your choice, not forge's. The point is about responsibility not security.

Someone places a random .jar into forge and have it executed. The only difference is that forge would accept an URL/URI and handle the download, while currently the mod itself would have to download it.

This is not what's in question at all, again faulty interpretation on your part. The question is about trust and responsibility not security.

It is literally a fig leaf.

I don't think you know what a fig leaf is.

And not even thinking that the whole point of forge is to execute code from dubious sources. So even without coremods, it is still the same issue.

It really isn't, and I get the feeling you don't understand the issue at hand in the first place.

But getting rid of core mods would mean, that forge would have to be more open to accept patches for more detailed hooks as well actually be more constraint about what is intended. So more docs as well as keeping them updated. And not just the "read the code and interpret it for yourself". But that does not seem like it was ever a high priority and if docs actually exist, they are often outdated or even contradictory with forge providing a more better method, but the deprecated vanilla method still exists and has to be implemented as the patches for the new one are incomplete..

Again which flies in the face of recent moves by Forge. This was literally discussed in a public steam a few weeks ago. With moves to make coremodding less prevalent came sympathetic moves to triage PRs in a more timely manner and be more open to working with the community on this. The promotion of mezz was a key part of that move.

Which most likely leads to forge should be more open for new hooks/etc (also AT could still exist). So coremods are not needed. But it should also be about mod development and loading. Not patching minecraft. That should be handled at a completely different level. So finally it is just a compatibility issue between different mod loaders and not offload it to hundreds or thousands of mods and leave them to solve every potential permutation of them.

Which is literally what was discussed. Maybe move outside your own assumptions and look at what's actually going on.

I doubt any customer would accept an excuse about "Look, we only provided a full blown scripting engine with root access and a TCP stack as part of the firmware, but we intentially did not add a HTTP lib to allow someone from executing a script to fetch further code from pastebin. Not our issue that some copy&pasted a malicious script with HTTP support into it.". There is no difference between the script having to implement their own HTTP lib or not. The whole point of forge is to do funny things and be not airtight to make the funny things possible.

The. point. is. about. responsibility. and. choice. Forge doesn't make that decision for the end user, the user must decide what to trust and what not to.

Did not some mods in 1.7.10 or 1.6.4 download their libs, iirc some codechicken?

Which is why I pointed out that trust is transitive. If you trust Mod A, you trust all of its actions, that might include deploading Mod B. But the trust relationship is still established wrt Mod A. If you do not trust Mod A, you do not install it.

@asiekierka said:
No, yueh. If you add every hook needed for every coremod ever, you'll just slow down and bloat up the game massively for the 95% of players who will never use a single one of them.

Not everyone needs a render engine patched up to support shaders. Not everyone needs a Chunk class capable of handling infinite heights.

Well said.

@yueh
Copy link
Member

yueh commented Sep 29, 2017

but this is going offtopic now ... and in regards to the other "abusable" stuff you are right ... i was just under the impression from what i have read about that, it really was a concern about security and responsibility

For example AE2 has a verry different tick behaviour. Nearly no tile does tick in the vanilla way. As having thousands of cables constantly tick, because some parts might have to be ticked, is just bad. Instead we use our own scheduler. Which that can easily cause sponge to start shitting bricks as they assume nothing can tick outside the normal ITickable#update()

No, yueh. If you add every hook needed for every coremod ever, you'll just slow down and bloat up the game massively for the 95% of players who will never use a single one of them.

I am not saying get rid of coremods at all. Only on the forge level. But it is certainly something for an ideal world and not going to happen.

Not everyone needs a render engine patched up to support shaders. Not everyone needs a Chunk class capable of handling infinite heights.

These are large engine changes. It would probably even break AE2 as we make some assumptions about a world never being higher than 256 blocks for various reasons. These are really things which should be dealt with at a higher level and not offloaded to every single mod dev. There is simply no way we could deal with someone having the idea of minecraft being the next best engine for their server management until someone from HR decides it would be an awesome team building feature to drop forge mods into it, have it explode and take their servers down. Of course exaggerated, but the point is that should be distinct borders between large changes and it is finally just a question of "Forge works with NextBestThing or not". Not mod by mod.

Again, you're utterly missing the point. Sponge must hook into those classes, this is the best way, it works reliably when coremodders stay within their remit.

My point is not about sponge having to hook into forge. It is about forge not documenting what are our remits. It is just as important to document the dos and the don'ts.

We work with the developers of Forge, we literally sit in each others private irc channels and discuss this shit all the time. Regardless of whether it's documented we are confident in our assertions.

This is the important point. It happens in private, without any public documentation. It would be so much easier, if forge would provide a better documentation about some best practices in a central location. Not wasting your time, my time and every other devs time due to having to discuss what is right and what not, without being able to say "rtfm".

If you install forge. You have chosen to install forge. Forge will never choose to install other agents without your consent. Now if you install Mod A, and Mod A takes some undesired action (such as formatting your hard drive, or downloading Mod B with does some nefarious action) then it's still your choice, not forge's. The point is about responsibility not security.

You are missing the point here. I am not suggesting forge should download anything on its own. It would only act as proxy between Mod A downloading Mod B. The user still has to decide to download and trust Mod A first. And forge could even go as far as displaying a "Mod A is missing B, do you want to download it from URL?" for an explicit consent and not some implicit consent by simply trusting Mod A. It would be even a better than now, if it would just display an URL to manually download a dep and not having to google for it and land on a questionable site. Certainly it is always about trust in the end. But forge could certainly help to improve it further.

Btw. it is not that I do not understand your position and I probably agree on most things. Should it not be obvious, my point about being so annoying/stubborn/etc is the missing documentation about it. Not about that it might be wrong or that I do not like X. The why is usually missing (and ideally not presented by some random guy claiming whatever. Even if it might be true)

@markhughes
Copy link

Really happy to see progress on this!! Thanks @yueh for your hard work, it's really appreciated. This is exciting, and I can't wait to see what comes next. 😃

@Mumfrey
Copy link

Mumfrey commented Sep 29, 2017

This is the important point. It happens in private, without any public documentation. It would be so much easier, if forge would provide a better documentation about some best practices in a central location. Not wasting your time, my time and every other devs time due to having to discuss what is right and what not, without being able to say "rtfm".

This is true. I don't dispute that half the time these issues are actually because the operational contracts are implicit rather than explicit. By the same token, even though gabi, zidane, myself might be "random guys" I hope it can be understood that we're approaching nearly all these issues with altruism, it's why PRs like this are made in the first place. We realise the lines are not clear, and want to actually help when a mod might cross them accidentally. In the short term for our userbase's benefit, but in the long term just achieving better stability in the coremod space overall.

I agree that more documentation is required, unfortunately it's not something that can be fixed overnight but good moves have been made in that direction, in particular with the Forge ReadTheDocs site which is now more open for external contributions.

You are missing the point here. I am not suggesting forge should download anything on its own. It would only act as proxy between Mod A downloading Mod B. The user still has to decide to download and trust Mod A first. And forge could even go as far as displaying a "Mod A is missing B, do you want to download it from URL?" for an explicit consent and not some implicit consent by simply trusting Mod A. It would be even a better than now, if it would just display an URL to manually download a dep and not having to google for it and land on a questionable site. Certainly it is always about trust in the end. But forge could certainly help to improve it further.

Again this is something that was discussed during the recent livestream (hopefully there's a copy of it somewhere, it was done on slowpoke's channel) because this kind of issue is exactly what was discussed, and honestly this is outside of Forge's remit but things are moving in the right direction with the recommendation to split off and sign coremod portions of mods. I still don't personally see any kind of forge-native deploading ever happening though. Either way, this point is kind of off-topic for the whole thread so I won't tug on it any further.

Btw. it is not that I do not understand your position and I probably agree on most things. Should it not be obvious, my point about being so annoying/stubborn/etc is the missing documentation about it. Not about that it might be wrong or that I do not like X. The why is usually missing (and ideally not presented by some random guy claiming whatever. Even if it might be true)

I get that, and I hope you realise that we're operating under a banner of enlightened self-interest, which is ultimately aimed towards improving the situation for mods and plugins across the board. The core of the sponge team were there for the #nextstep discussions and we were there because we wanted to provide a route forward following the collapse of CB. None of us are raging egomaniacs and we've worked closely with Forge from day zero. I realise it's impossible to seem impartial and you can interpret our biases whatever way you choose. I think the issue is that we might all be some random guy claiming whatever, but I hope at least from reputation that you know we're coming at issues with the right mindset, even if there isn't the documentation there to back up our assertions.

Maybe our approach on these issues should be to focus on getting our points underwritten by getting documentation added to Forge instead of making issues like this. I mean that would ultimately benefit everyone were that the case since then - to use your words - we literally could just say "rtfm".

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests