Skip to content

[1.9] Loot hooks#2543

Closed
williewillus wants to merge 2 commits into
MinecraftForge:1.9from
williewillus:loothooks
Closed

[1.9] Loot hooks#2543
williewillus wants to merge 2 commits into
MinecraftForge:1.9from
williewillus:loothooks

Conversation

@williewillus
Copy link
Copy Markdown
Contributor

@williewillus williewillus commented Mar 14, 2016

Preliminary draft:

Done so far:

  • Loading from mod jars (with overrides)
  • Event fired when Loot Table has finished populating a list of items.
  • Patch in a getWorld into LootContext. If the context doesn't involve a living thing killing another living thing, then it's impossible to get the world. Useful for chests in dungeons.
  • Tests to test the above

Not done yet:

  • Method and design of loading needs to be refined (I just went with loading from jars + a override mechanism suggested by @diesieben07)
  • A way to modify existing tables
    • Event fired in LootTableManager.Loader.load()?
    • Should world-specific override tables obey changes made during the event or not?

I thought I'd start off the discussion for this, since it's one of the parts I'm most interested in in 1.9.

Some terminology

Ctrl+F "Loot tables" in this for some basic terminology
https://gist.github.com/williewillus/e37edde85dc78d2e138c

Adding pieces to the looting system

  • All the abovementioned classes are easy to add to in code. Each class has its own registry where you can register your object to a name, e.g. I can register a LootFunction "botania:add_mana" that takes a stack and adds mana to it simply by calling LootFunctionManager.registerFunction(Serializer). For all of these, you have to pass in a serializer describing how it'll be (de)serialized from JSON form.

Activation of the looting system

  • For living entities: The entire loot table system is only queried for subclasses of EntityLiving. EntityLivingBase#onDeath calls dropLoot, which is fully overriden by EntityLiving. dropFewItems doesn't do anything anymore (except for slimes which still use it for some reason.
    • The loot table to use is an NBT tag in the entity, "DeathLootTable". Falls back to a default for vanilla mobs.
    • The forge hook to capture drops is in EntityLivingBase#onDeath and should still capture all loot dropped. Perhaps we should provide the loot context in LivingDropsEvent, though that would require us to move the event. A more viable alternative is to expose the LootContext in an event, allowing others to modify the Loot Tables in code?
  • For TE's and things like minecarts. These implement ILootableContainer, which contains a single method returning the ResourceLocation of the loot table to generate. TE's and carts have a field/NBT tag that holds this ResourceLocation. Upon the first opening of the container, the loot table is queried and then the loot table field in the object is set to null. It's possible in code for the loot table to be re-set so that it regenerates again whenever the container is opened next. I don't think this requires any hooking/modification.

Loading of loot tables

Probably the most difficulty piece to figure out currently.
Although loot tables in the vanilla jar are in the "/assets/" directory, they are far from being part of the powerful cascading resource pack / clientside asset system. What vanilla does currently in LootTableManager.Loader.load(ResourceLocation) is look for loot jsons in the world directory (new File(LootTableManager#baseFolder, ...)), a vanilla feature allowing mappers to override tables, and then fall back directly to the classpath (mod jars) if the former is missing (LootTableManager.class.getResource(URL)), then to an empty table if it's still missing.
The easiest solution is simply to place a hook directly below the loading from world folder call, that loads from mod jars, but still lets mappers' world-specific tables have top priority
The question arises is that with this, we don't know what jar to look in. Looking at the resource domain of the resource location to determine which jar to search would be a fallback but it would prevent an addon for example from changing its parent mod's tables. Or we could scan through all jars to look for any loot json, but that implicitly enforces an ordering in the case of conflicting jsons.
(Or we could find a way to bring that whole cascading resource system to the serverside? I don't know that system well enough to say if that's feasible)

Alternatively we could just forego this entirely, use the vanilla registries, and just have mods "figure it out" how to get an instance of LootTable to the registry. (i.e. they handle deserialization themselves). But that's meh.

Thoughts?

What would get obsoleted

  • ChestGenHooks and FishingHooks are both going to go away
    • This requires us to determine a way to inject mod entries into the main loot tables. Can't replace the tables because then mods would conflict. Would using the events proposed above be suitable?

* Add your own loot pool to the Loot Table.
* @param pool The pool to add to the list of pools.
*/
public void addPool(LootPool pool)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not expose the pools list instead? Doing so will allow clearing, removing, adding etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not exposed for removal because Loot Pools have NO identifying information whatsoever. You'd have no idea at all what you're removing
This is the part I need to figure out, because for a true alternative to the old hooks ee need to be able to merge pools together, not just tables

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may not be able to identify them easily, but it's still nice to be able to control the list directly. addAll, clear, etc methods are available there, whereas there's just an add method here

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, we can FORCE mod ones to have unique identifiers, and we should.
As for vanilla we can fake these unique identifiers quite a bit.

@williewillus williewillus changed the title Loot hooks [1.9] Loot hooks Mar 20, 2016
* Call this anytime during startup.
*/
public static void registerOverride(Function<ResourceLocation, InputStream> override)
{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What prevents ModA taking over loot tables for ModB directly?

@williewillus
Copy link
Copy Markdown
Contributor Author

Yeah not sure where I wanna go with this :P
Any other suggestions or attempts to do this are welcome

@williewillus
Copy link
Copy Markdown
Contributor Author

So reading through the vanilla spec, I just realized that a loot entry can simply point to another loot table.

So my idea is, at least for the time being so we get feature parity again, is that on startup modders call some registration method and specify the Loot Table + Loot Pool that they want to add an entry to, as well as the resource path of their loot table. Forge will than inject another loot entry that points to the modder's loot table.

@LexManos
Copy link
Copy Markdown
Member

The issue is still uniquely identifying the loot pools/entries in the cases where vanilla specify multiple of them {there are like 5 cases}
Pointing at another loot table is the easy part because we just make it's id the resource location of the pool.
It's the Item entries and the multiple pools that are the issues.
And then the only real api we need to expose is:
To LootTable
LootPool getPool(String ID)
void removePool(String ID)
To Pool:
get/setRolls
get/removeEntry(String ID)

As any entry that is a table is just a link to another table not it's own instance, we don't have to care about recursion.

tables.get('minecraft:fishing').getPool('main').getEntry('minecraft:fishing/treasure') is the same exact result as tables.get('minecraft:fishing/treasure')
Any edit to one edits the other.

Anyways what i'm thinking is two processes:
We scan each mod jar in load order for 'assets/{domain}/loot_tables/{name}.json'
For each of those entries that are NOT already registered we register them.
This is the mechanic for creating new tables.

To edit existing ones, im not to sure. We could either do it in code 'LootTableLoadEvent' and let them do their own modification in code which SHOULD look something like:
e.getTable().getPool("main").removeEntry("minecraft:fishing/treasure") // Remove treasures
e.getTable().getPool("main").addEntry("coolfish:fishing/extra_fish") //Add more fish as an external entry

And then something like this for removing/adding entries for items:
e.getTable().getPool("main").removeEntry("minecraft:fish.1") // First fish item entry, as vanilla doesn't use unique ID's we'd have to hard code this at inital load time before we load any overrides
e.getTable().getPool("main").addEntry(MY_POOL.getEntry("coolfish:eel"))

Now, what happens if ModA removes "minecraft:fish.1" does "minecraft:fish.2" becomes .1? No, These IDs are static and will be generated at the beginning.
Any MOD table that DOES NOT specify these IDs will throw a FATAL ERROR. I'd be okay with crashing the game with a valid error for invalid data. {The other option is to return a empty table if we wanna display but handle the error}

Just some things i've been spit-balling around. Think you could poke at it willie?

@williewillus
Copy link
Copy Markdown
Contributor Author

Mmm that seems like a sound idea

Idk if I can do it though, I want to work on getting my PIE PR in, and between that and Botania and ProjectE and school I probably don't have time to spend on loot tables much (I don't really need it that desperately, really).

If other modders need it more desperately feel free to whip up your own impl based on what Lex has said above, feel free to steal code from this impl.

@williewillus
Copy link
Copy Markdown
Contributor Author

superseded

@williewillus williewillus deleted the loothooks branch May 14, 2016 21:59
Yopu referenced this pull request in Railcraft/Railcraft Jun 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants