RubilaxXxx / ArsMagica2-5

Ars Magica 2.5
5 stars 1 forks source link

AM2 modifiers metadata is (guess what again) FUCKED, and AM2 spell components metadata is generally FUCKED A LOT #16

Open AirBurn0 opened 1 month ago

AirBurn0 commented 1 month ago

WARNING! Lots of shit below!

Maybe you're not the one who should fix this. Maybe I should do this. But, hey, guess what:

Modifiers

Problem

Ever tried to make [Projectile] [Projectile] [Projectile] and making every color different? Yes I know that it sounds stupid but hey, just try it (and please remove AM2PG if you have one, that will ruin the magic). Just like that: image try to guess what colors projectiles will have.

Answer Of cource AM2 does not give a fuck and all projectiles will be RED!

Source of problem & explanation

AM2 invokes spell using Stack-like approach: https://github.com/RubilaxXxx/ArsMagica2-5/blob/5536709fac7860d7cd8aee9cc4adb94f07f853e1/src/main/java/am2/spell/SpellUtils.java#L615-L634 And since spell NBT kinda looks like this: image So AM2 really should have some code that will pop "SpellModifierMetadata_"-tags but it refused.

There is how I solved that (just injected with ASM lol):

public static void popModifiersMetadata(ItemStack stack, int stage) {
  NBTTagCompound t = stack.getTagCompound();
  int[] ms = t.getIntArray("SpellModifierIDs_" + stage);
  if(ms == null) {
    return;
  }
  HashMap<Integer, Integer> ordinals = new HashMap<>();
  for(int m: ms) {
    m -= SkillManager.MODIFIER_OFFSET;
    int ordinal = ordinals.getOrDefault(m, 0);
    String oldKey = "SpellModifierMeta_" + m + "_" + stage + "_" + ordinal;
    byte[] meta = t.getByteArray(oldKey);
    if(meta == null) {
      meta = new byte[0];
    }
    t.setByteArray("SpellModifierMeta_" + m + "_" + (stage - 1) + "_" + ordinal, meta); // SAME array
    t.removeTag(oldKey);
    ordinals.put(m, ++ordinal);
    }
  }

And it kinda works (install AM2PG and check if you doubt).

So.. Problem solved?

NO.

Mition is not one of those who fucks up only once. SpellUtils.createSpellStack() is kinda fucked up too. Now, if you were told that there was a stupid error in this code, how quickly would you find it? https://github.com/RubilaxXxx/ArsMagica2-5/blob/5536709fac7860d7cd8aee9cc4adb94f07f853e1/src/main/java/am2/spell/SpellUtils.java#L601-L606

Answer `ordinal++` that part. Know why?
Answer Because `ordinal` value is NOT used out of scope, and every scope loop it resets to what it originaly was. It takes 0, puts it into HashMap, increments useless now value by 1, and then on a new loop step it just reads it like "oh hey, I wrote 0 here, so you can just have it back", and does exactly the same - puts 0 into HashMap!!!! Any ideas how to solve it?
Easy, if you ask me change `ordinal++` to `++ordinal` or `ordinal + 1`, lol.

Well, that's it now? Nope. But this one is simpler: https://github.com/RubilaxXxx/ArsMagica2-5/blob/5536709fac7860d7cd8aee9cc4adb94f07f853e1/src/main/java/am2/spell/SpellUtils.java#L572-L578 ordinals map is being readed, but never being writed. so after calling addModifier() (line 577), just add that line:

ordinals.put(modifier.getID(), ordinal + 1);

And hope for problem solved!

Mithion fuckup-in-single-place counter: 3 (new record!)

Components and shapes

Now when you know how Modifiers metadata system SHOULD REALLY WORK, it's time to introduce Components metadata! Unfortunately, the AM2 code only hints at such a system, but it doesn't actually exist... Until AM2PG. You know what's commonly bad for [Binding] and (Summon)? Making any spell with more than one [Binding]/(Summon) like [Projectile] (Summon) (Summon) will basically summon 2 identical creatures and AM2 basically doesn't give a fuck (again, lol) if you used different phylacteries. This happens because [Binding] and (Summon) data is shared globally across entire spell, that's the result of not implementing Components and Shapes metadata. This results in sometimes more crazy bugs than I descrubed here (Like, you in fact can make tool from every existing spell, if you use [Projectile] [Binding] and like switch for another spell before projectile hits). But hey - AM2 doesn't allow you to create [Projectile] (Summon) (Summon), and spells like [Binding] [Projectile] [Binding] does not have any sense! Well, fair point, but know why it is forbidden or not making any sense? Cuz of stupid API. Let's head for the example of what can possibly be made if such a system was implemented - hopefuly I do implement it in AM2PG: (Fireworks) image This thing is basically imitates Firework Rocket explosion behaviour, and was impossible to do nicely with current state of AM2.

Well, I didn’t come here to brag, but to ask to add components and shapes metadata into AM2, otherwise it’s somehow shameful. All code for components has been already written by me, but since AM2PG is not open-source, just eh... Do you want to have some AM2PG code? The real request here is to rewrite [Binding] and (Summon) into new system, including removal of all artifacts and crutches.

Summary

I explained what's wrong with the current spellparts metadata system but don't really sure about who should implement it - generally speaking this is my job, since I have all the code, but I don’t think I’ll be doing this in the near future, so go ahead if you want, I'll try to provide all necessary code if needed.

RubilaxXxx commented 1 week ago

Ok so here's my 3 "big" change to spell creation i'm gonna do.

1.remove stages, will simplify a lot of stuff code wise aswell as making everything clearer.

2.limit shape to 1 per type (like components) so no more proj, proj, or whatever else. with this i'll increase the shape box length to accept more shapes.

  1. I don't remember :p.