MerchantPug / apugli

An extension to Apoli for certain mods' use.
MIT License
5 stars 4 forks source link

`apugli:modify_enchantment_level` Power effecting `apoli:enchantment` Item Condition #31

Closed JustCyra closed 1 year ago

JustCyra commented 1 year ago

When apugli:modify_enchantment_level Power modifies specified enchantment levels it also modifies the apoli:enchantment Entity Condition which is perfectly fine, however somehow this also got extended to the Item Condition version. Checking for that Item Condition on items gives a wrong result.

To test use the Example Power and equip anything on the Head slot with minecraft:respiration level of 1. The condition checks for if the equipped item has 2 or more levels of minecraft:respiration however due to the apugli:modify_enchantment_level Power modifying that condition too it gets activated if you have 1 or more levels instead due to the power adding 1 extra level as base.

Minecraft 1.19.3 Fabric Loader 0.14.19 Origins 1.8.1 Apugli 1.13.3

Example Power:

{
    "type": "apoli:multiple",
    "main": {
        "type": "apugli:modify_enchantment_level",
        "enchantment": "minecraft:respiration",
        "modifier": {
            "operation": "add_base_early",
            "value": 1
        }
    },
    "test": {
        "type": "apoli:action_over_time",
        "interval": 10,
        "condition": {
            "type": "apoli:equipped_item",
            "equipment_slot": "head",
            "item_condition": {
                "type": "apoli:and",
                "conditions": [
                    {
                        "type": "apoli:empty",
                        "inverted": true
                    },
                    {
                        "type": "apoli:enchantment",
                        "enchantment": "minecraft:respiration",
                        "comparison": ">=",
                        "compare_to": 2
                    }
                ]
            }
        },
        "entity_action": {
            "type": "apoli:execute_command",
            "command": "say Bugged! Condition requires 2 or more level of 'minecraft:respiration' on 'head' slot!"
        }
    }
}
MerchantPug commented 1 year ago

This is a known issue that unfortunately cannot be changed. This is because of how the condition works.

Theres only one way of getting the player's enchantment levels in Minecraft because the game doesn't account for non nbt enchantments as it doesn't need to.

MerchantPug commented 1 year ago

If Modify Enchantment Level ever comes to Apoli, I may modify this behavior, I would prefer not to for Apugli.

MerchantPug commented 1 year ago

For those wanting to solve this, you might be able to check NBT on the item, at the cost of a little performance.

Only do the above if the power isn't a flat modifier, if it's flat just add the amount to the condition.

MerchantPug commented 1 year ago

I've had to reopen this issue because I've realised that a mixin into both enchantment conditions was required to solve #21 properly.

The original two conditions will directly get the power sensitive enchantments but a new entity/item condition will be available for getting the original tag enchantments.

I plan on PRing Modify Enchantment Level to Apoli at some point, as I've managed to figure out the proper implementation for this power and it'd be a lot easier to maintain this power over there.

There will not be two conditions when it comes to the Apoli implementation, and it will instead contain a new field that activates either check style.

MerchantPug commented 1 year ago

Worked around in v2.0.0