New Furnaces, improving how furnaces are added

[plnx]

This might belong in "Forge Suggestions", but I feel too new to be sure of what I'm saying, and am looking for some more general comment.

 

Just as an excuse to explore the code, I wanted to add a new type of furnace, and found even if all I wanted was for it to appear differently, it isn't a trivial exercise. Before I read it, my first thought was to sub-class BlockFurnace (basically empty, just to have something to hang new asset files on) but ran first into the call to the static method setState in BlockFurnace causing my furnace to be replaced with a vanilla one when activated in-game; and shutting down my hopes of inheritance (among other points I then noticed that would make it impossible).

 

Poking around git hub, seems the going strategy instead is to sub-class farther up, resulting in a ton of copied code handling the common furnace logic.  Some people are even then duplicating it repeatedly for every new furnace they add, making it worse.  I know we're making a system do things it's designers didn't intend--this sort of thing is probably unavoidable at times--but it still just feels so wrong, especially for something I thought would be a basic change.

 

So...

 

[*]Should I work towards a contribution to Forge?  I have a feeling there's some issues I'm not aware of causing this to not already be in place.  (and I still have a lot to learn before I'm ready for that)

[*]Is there a library already out there that would at least does this duplication only once for all mods that use it?  Is there a reason why this would make more sense as a library, as opposed to, say, new event hooks for Forge?

 

Thanks.

[Draco18s]

[plnx]

Ha, as much as I'd agree every topic could use XKCD, I'm not advocating for a standard, and I didn't notice any competing "standards" (if by that you mean libraries) out there.  With the little experience I have around here, I'd be leaning towards the forge solution anyway--I just don't know what that means yet.

[TheGreyGhost]

Hi

 

Although I like the idea, in my opinion the last thing Forge needs is more specialised features that seldom get used.  It's groaning under the weight of all the "nice to haves" already, and every extra one just adds to the maintenance burden and slows down each update.

 

I'd suggest you make a good example and let folks copy it.  They'll need to update it for every minecraft update, but I think that work is better placed on the shoulders of the individual developer rather than Lex & co.

 

-TGG

 

[Draco18s]

Pretty much unless Forge needs to be doing something, it should be left up to the individual developer.  Liquids being one example: there needs to be a single way to go "oh that's a liquid" so the various mods that have pumps and pipes can handle every mod liquid the same way.

 

Furnaces?  Not really.  There's a single furnace recipe repository and that's really all that's needed.  Various mods will do their own cooking their own way as their specs require.  Some mods might use different fuels, cook faster, cook slower, have a variety of other tweaks and alterations.  There's no "good" way to supply a common package for this.

[plnx]

Thanks guys,  both those comment help.  As an aside, being new around here, I'm often not sure when I'm looking at examples and tutorials if what I'm seeing is "crazy" because it has to be, or because the author doesn't know any better.  Don't get me wrong, I've see a lot of good stuff, but I also get the sense many people get into this with no prior development experience; and often tutorials will tell you what to do, but not why it has to be that way.

 

Anyway, I suppose I could argue what is deemed "needed" can be subjective (maybe I want "oh that's a furnace", and don't really care about liquids--just playing devil's advocate), or that one of forge's design goals in general should be to avoid plug-in authors duplicating code as much as possible (as in the case, "I want this to behave exactly like the vanilla version, even if the MC changes", not like trying to predict all bespoke variations of that logic, in which case I'd totally agree you should be on your own) ... but I take your points that this specific case is really pretty niche, I'd call it only "nice to have" too,  and it make sense that forge should keep it's patches as minimal as possible or risk becoming unmaintainable.  When I'm farther along, maybe I'll write that "common package" (or "good example"), something along the lines of duplicating TileEntityFurnace and children only with relaxed protections, more setters and so on; but for now thanks for helping me get my head on straight.

[Draco18s]

Anyway, I suppose I could argue what is deemed "needed" can be subjective (maybe I want "oh that's a furnace", and don't really care about liquids--just playing devil's advocate), or that one of forge's design goals in general should be to avoid plug-in authors duplicating code as much as possible

 

Really though, there doesn't need to be a common furnace.  It's a TileEntity (common code) that implements ISidedInventory (probably) or at least IInventory.  Which is all that's needed for every mod ever to be able to insert items or extract them (including using the vanilla Hopper).  No other mod needs to care about how your CustomFurnace actually smelts inputs into outputs, so having a "common core" for the updateTick method is pointless.

 

Per liquids, I suspect it was originally because creating a new liquid was Hard* and so it was written as a base class that mods could extend in order to easily get a liquid working "like vanilla" and morphed later into the liquid registry (et cetera) due to the number of mods wanting to move liquids around and treat them in common ways without having to do a bunch of special casing.  There is IFluidBlock, which is implemented by BlockFluidBase, but it doesn't really supply all of the necessary details, as BlockFluidBase is the class that handles the Fluid object.

 

But sure, feel free to create that library if you want to.  Dollars to doughnuts no one will use it.

 

*My first attempt at a liquid, using the Forge liquid base block didn't even work correctly, just to give you an idea of how Hard it actually is!

[plnx]

> But sure, feel free to create that library if you want to.  Dollars to doughnuts no one will use it.

 

Eh, you're probably right, and I'm not that dead-set on it anyway.  Really just feeling things out at this point.  (and, hm, I can't seem to work the "insert quote" link)

 

But, pretending I am, allowing other mods to care about how a furnace smelts wasn't my point.  More like, allowing mods (and MC itself) just to know this thing is a furnace, however it works (like I said, "not like trying to predict all bespoke variations of that logic..").  Is that very useful, particularly compared to something like liquids?  Nah, probably not.  As for the library, I meant that less about providing a common furnace than giving people a base to extend, save some time.  Going back to my original example of a furnace that just looks different (it was a sandstone furnace), it seems kinda silly that I have to repeat all these classes just to do that (though I understand the reasons).  Even if no one else used it, if I was creating a mod that added a bunch of furnace, such set of classes would be my first step anyway.

[Draco18s]

If you (generic) are creating a furnace that "just looks different" you would just have all the Blocks create the same TileEntity.  There's no reason to create a new TE class (even one that extends some generic version) just because it belongs to a different block.

 

As for "allowing mods (and MC itself) just to know this thing is a furnace": what value is there in that?  Why "furnace" instead of "grinder" "machine" "oven" "a thing that has an inventory and updates every tick and alters that inventory" or some other niche distinction?  Why is "furnace" the important distinction to make?

[coolAlias]

The problem lies in the TileEntityFurnace having hard-coded references to the vanilla active/idle furnace blocks, as well as using a static method 'keepInventory' - if this weren't the case, every modder could easily extend this one class, make a few minor changes and be done with it.

 

This goes for any type of machine - I've seen (and coded some myself... derp) many machine variations, grinders, furnaces, etc. that all follow the vanilla 'model' and make the same poor design decisions, but even if you were to design a more flexible class, one that say takes two Blocks in a non-static keepInventory method and by default switches between the two, what if a mod wanted 3 states? Or 4?

 

Also, the logic within the TE class itself is always going to be far too specific, whereas the fact that it implements one of the Inventory interfaces already makes it generic enough for any mod to interact with it (as Draco mentioned). The best I've come up with, not really related to furnaces per se, is to make a generic TileEntity with IInventory implementation, since I found myself repeating that code a lot.

 

If I was making lots of furnaces and other machines, I might be inspired to do something similar, but I don't think it would turn out to be as widely applicable as one might hope.

[plnx]

Thanks again guys, this is all basically what I'm saying too, or would have said if I had more experience here.

 

... you would just have all the Blocks create the same TileEntity.  There's no reason to create a new TE class ...

 

Right, that's the error I was pointing out when I said "Some people are even then duplicating it repeatedly for every new furnace they add..."

 

... As for "allowing mods (and MC itself) just to know this thing is a furnace": what value is there in that? ... Why is "furnace" the important distinction to make?

 

Again, it's not, I said it's probably not very useful.  Creating a "common furnace" was a tangent I didn't mean to get so far into; this was all just an excuse to cut my teeth, I personally don't actually care that much about furnaces.

 

The problem lies in the TileEntityFurnace having hard-coded references to the vanilla active/idle furnace blocks ...

 

Yep, it's in that static method setState I mentioned.

 

if you were to design a more flexible class, one that say takes two Blocks in a non-static keepInventory method and by default switches between the two, what if a mod wanted 3 states? Or 4?

 

I wouldn't be trying to make something more flexible, I was only thinking about "relaxed protections, more setters and so on", in other words just the vanilla class, refactored to allow people to extend it (this is why I was at first thinking maybe this is something forge should patch).  If you wan more than one slot, ovverride.  If that means you'd need to basically override everything anyway, then right, not for you.

 

Well, I'm satisfied, and have some more specifics to look into now.  Just to be clear, I'm not arguing for a common furnace, and am not about to embark on some kind of Furnace Makers API. I guess I don't even know the right way to ask questions yet... turns out what I really got from all this was a better grasp on the architecture in general.