Astral Sorcery

Astral Sorcery

63M Downloads

Stray code in other json files from AstralSorcery

remclave opened this issue · 9 comments

commented

Not causing a crash but this code shows in every entry of our BetterQuesting mod DefaultQuests.json file:

        "ForgeCaps:10": {
          "astralsorcery:cap_item_amulet_holder:10": {}
        },

As large as our quest book is, that is a LOT of extraneous lines of code eating up filespace. What is the purpose of this code, and can it be removed without consequences to Astral Sorcery installation in the modpack?

ETA: we do not have a quest line for Astral Sorcery in our quest book.

commented

It's for things the amulet might be able to modify enchantments on, and should only be applied to items that are unstackable. Can you provide some more context for appearance?

edit: also, what version of AS?

commented

A.S. version 1.10.3 (we just updated).
This entry is for the Thaumometer from Thaumcraft 6. This is the entry in its' entirety:

"20:10": {
  "questID:3": 20,
  "preRequisites:11": [
    19,
    216
  ],
  "properties:10": {
    "betterquesting:10": {
      "issilent:1": 0,
      "snd_complete:8": "minecraft:entity.player.levelup",
      "lockedprogress:1": 0,
      "tasklogic:8": "AND",
      "repeattime:3": -1,
      "simultaneous:1": 0,
      "icon:10": {
        "ForgeCaps:10": {
          "astralsorcery:cap_item_amulet_holder:10": {}
        },
        "id:8": "thaumcraft:thaumometer",
        "Count:3": 1,
        "Damage:2": 0,
        "OreDict:8": ""
      },
      "globalshare:1": 0,
      "questlogic:8": "AND",
      "partysinglereward:1": 0,
      "snd_update:8": "minecraft:entity.player.levelup",
      "autoclaim:1": 0,
      "ismain:1": 0,
      "name:8": "Just Like Sure Luck Homes...",
      "desc:8": "Craft the Thaumometer on the Arcane Worktable. \n\nUse the Thaumometer to examine everything in your environment that sparkles. The various mobs in your environment are scannable as well. Any environmental object that is scannable will sparkle when you hover the Thaumometer over it. To learn what you can, scan it by right-clicking. To learn objects in your inventory, drop them on the ground. If it sparkles, you can learn from it. Scan it. §l§eAlternatively, place all objects into a chest and shift-right-click the chest.§r§r This is a great way to keep any clear lag mods from removing your items while trying to scan them...\n\nSome scanning will provide Notes for Theorycrafting. Keep Paper and Scribing Tools in your personal inventory.\n\nAs you gain knowledge, some items that were not scannable the first time will possibly be scannable on a later pass."
    }
  },
  "tasks:9": {
    "0:10": {
      "partialMatch:1": 1,
      "autoConsume:1": 0,
      "groupDetect:1": 0,
      "ignoreNBT:1": 0,
      "index:3": 0,
      "consume:1": 0,
      "requiredItems:9": {
        "0:10": {
          "id:8": "thaumcraft:thaumometer",
          "Count:3": 1,
          "OreDict:8": "",
          "Damage:3": 0
        }
      },
      "taskID:8": "bq_standard:retrieval"
    }
  },
  "rewards:9": {
    "0:10": {
      "rewardID:8": "bq_standard:item",
      "index:3": 0,
      "rewards:9": {
        "0:10": {
          "id:8": "thaumcraft:curio",
          "Count:3": 1,
          "OreDict:8": "",
          "Damage:3": 2
        }
      }
    }
  }
},
commented

The thaumometer has no enchantments that I know of...
But the "unstackable" part is true.

commented

Yup, so it's a valid target for that capability nbt tag

commented

Does the capability data absolutely need to be there if the item has yet to be modified by such amulet. It's effectively empty until used anyway right?

commented

Without going through and maintaining a special casing set for every single item in every single mod ever released, and updating it properly, yes. This is still the most effective and efficient method to do so.

You could attempt to strip it out for the quest, but if it's looking for matching, it may not work. I don't know how the quest book does NBT matching, but I willing to bet it's not doing it in a way that makes sense

commented

The ultimate problem for this is i have to have a way to resolve the player holding/wearing an item only given that specific itemstack. And no other information or context data. Adding the information who's wearing/holding the itemstack is a critical point. If you have a better solution, let me know. But the matching itemstack -> player MUST be there.

Can't go with direct object comparison with == because the itemstack gets cloned in the process and thus the references don't match anymore.
Comparing every property of the itemstack is problematic because let's say 1 player with a non-enchanted diamond sword gets an amulet-enchantment effect, then all players holding an non-enchanted diamond sword would get that effect.

I could list other problematic scenarios but that's the gist of it. Perfect ItemStack -> Wearer matching must be provided.

commented

Okay, the information provided is above my level of understanding. As the mod author of the affected mod also replied, I presume he understands what is happening and I will leave it up to him and you to work out what should be done. I will still be on the sidelines and watching, hopefully learning how this stuff works...

commented

It can be manually stripped out with any adverse side effects to stack matching. BQ has it's own custom NBT matching that includes partial and out-of-order tag lookups which I've tested.

I'll just leave it at that for now seeing as this isn't strictly a bug or anything game breaking. I do wonder why the capability needs to be persistent even though it seems to update itself every tick while worn or why the stack is in charge of identifying it's owner rather than the other way around. Anyway, you can close this issue if you like (you still have me on Discord if you want to ping my about this more).