ThauEx / ffrk-proxy

Proxy for Final Fantasy Record Keeper written in node.js
GNU Lesser General Public License v3.0
39 stars 21 forks source link

Feature/Bug Elemental Attacks #25

Closed SirPhoenix88 closed 7 years ago

SirPhoenix88 commented 8 years ago

I was wrong. It wasn't the infinity abilities taking away the "weakness" indicator for the enemies. Something in the current code removes all elemental resistances AND vulnerabilities for the enemy.

I will take a look at the enemy code and see if I can improve it any.

SirPhoenix88 commented 8 years ago

json[round].enemy[enemy].children[part].params[0].def_attributes = [];

It appears this lines removes resistances, immunities, and vulnerabilities. If intentional, can leave, or maybe put some logic to check if we are strengthening the monster by removing it's vulnerability.

ThauEx commented 8 years ago

This line is there since the beginning... So I'm not sure, if this is the issues. Maybe dena changed something in the code.

SirPhoenix88 commented 8 years ago

I usually turn status boosts/debuffs off, so I may have had it turned off before we did all of our updates. But I pulled a normal block of code, and it seems that when the attribute_id is set to a "factor" of 1, the enemy is weak to the element, but if the "factor" is 11, it would likely nullify or absorb the element.

If we want to make all enemies vulnerable to all statuses, we can leave it as is. I just felt that removing status vulnerabilities was unintentional.

SirPhoenix88 commented 8 years ago

I tried to improve upon it by checking if it had an elemental attribute (less than 200) and then if it was a weakness, then deleting the attribute if it satisfied none of these conditions, but it had no effect.

      for (var att in json[round].enemy[enemy].children[part].params[0].def_attributes) {
          if (!(att.attribute_id < 200 && att.factor == 1)) {
              att = "";
                      }
      }

Any suggestions on how to improve it? If it's an element AND a weakness, I don't want to delete, but for all else, delete the attribute.

ThauEx commented 8 years ago

When you disable def_attributesit is working fine again?

for (var att in json[round].enemy[enemy].children[part].params[0].def_attributes) {
  var attributes = json[round].enemy[enemy].children[part].params[0].def_attributes[att];

  if (!(attributes.attribute_id < 200 && attributes.factor == 1)) {
    delete json[round].enemy[enemy].children[part].params[0].def_attributes[att];
    // delete attributes; // maybe this works too
  }
}

Can you test this code?

SirPhoenix88 commented 8 years ago

I can in half an hour. Low on stamina because I just finished Penelo's events this morning, after using all my weekend stamina on Orbfest.

SirPhoenix88 commented 8 years ago

Didn't work. I'm testing different syntaxes with splice and other logic.

KHShadowrunner commented 8 years ago

Wouldnt this imply that if we can remove waknesses and resistances, we can implement them? Not that it matters, but we could make them weak to.. well.. everything by simply adding rather than leaving it blank.

Could be wrong though, may take a look if there's interest.

SirPhoenix88 commented 8 years ago

We could implement that. I was trying to simply leave the weaknesses and pull away that status resistances, but my logic was coming up undefined for all comparisons. I need to figure out something new, but my new job started so my time is much shorter.

SirPhoenix88 commented 8 years ago

This also gives me a new idea. If we apply this to buddies, we can set status immunity to... everything. "Ribbon" Feature!

KHShadowrunner commented 8 years ago

So I think the best way to get around it would be to add the items to a second array and then redefine the first array as the second one. it'd be like:

`var NewDefAttrib = []; for (var att in json[round].enemy[enemy].children[part].params[0].def_attributes) { var attributes = json[round].enemy[enemy].children[part].params[0].def_attributes[att];

if (attributes.attribute_id < 200 && attributes.factor == 1) { NewDefAttrib.push(attributes); } } json[round.enemy[enemy].children[part].params[0].def_attributes = NewDefAttrib;`

Or something like that. I'll give it a try.

KHShadowrunner commented 8 years ago

Looking at the dumps from another program after the changes, this code works:

` if (config.get('enemy.defAttributes.enabled')) { //currentEnemy.params[0].def_attributes = config.get('enemy.defAttributes.attributes');

      var NewDefAttrib = [];
      for (var att in json[round].enemy[enemy].children[part].params[0].def_attributes) {
        var attributes = json[round].enemy[enemy].children[part].params[0].def_attributes[att];

        if (attributes.attribute_id < 200 && attributes.factor == 1) {
          NewDefAttrib.push(attributes);
        }
      }
      currentEnemy.params[0].def_attributes = NewDefAttrib;

`

But the display on the cmd doesn't work appropriately. I dont know the weaknesses, so i can test anything. but (actually I have a way of at least confirming what it's weak to, but testing it may take a little bit).

EDIT: Just tested and can confirm that it shows Vulnerable

KHShadowrunner commented 8 years ago

I have no idea how you want to handle fixing the "Extra" column. Best guess is to change it from "Extra" to "Weaknesses". I added the 8 element types into the attributes.js, but you have a conflict with Poison (which is both element - 108 and status - 200), but I'm not referencing them. We could call to them to compare the ID of the element into the table and add the name to Buffs, maybe?

KHShadowrunner commented 8 years ago

So for displaying this, I've been trying to think of the most elegant way to do it. The only way I can think of would be making a new function that checks either a new JS or the ailments JS if the items are added, and scans it to match the IDs - then pull the name. I tried to code this, but started really messing things up, so I stopped for a little bit.

If anyone has a suggestion on how to basically scan (within the in loop you could pull attributes.attribute_id) the JS for a match to the Id and pull the... key? I think? that might work in clearing up the table.

KHShadowrunner commented 8 years ago

Since this is listed as a feature, I figured I'll add this in here. It's possible to turn any added attack via materia (ie - if a materia has an attack addition) to be elemental. Helpful for targets on bosses?

KHShadowrunner commented 8 years ago

I'd like to try and add the elements to the ailments.json file and this wouldn't be a problem with one exception - Poison.

how do we want to handle the difference between the element Poison and the status Poison? I can just call it Bio I suppose? Looking for feedback - I'm going to try and make a new pull request to better handle adding the Attack and changing it to either multi-hit (in addition to hit all?) or Element if set.

ThauEx commented 8 years ago

Do we need poison? Atm normal attack already have poison.

KHShadowrunner commented 8 years ago

Sadly, there is a difference in terms of meeting boss conditions with:

I'd imagine just using Bio as the name for the elemental weakness would be fine - but that's your call

EDIT: The difference is the difference between hitting with the spell Poison, or hitting with Bio/Bioga

KHShadowrunner commented 8 years ago

As for the code fix:

the delete function wont work on objects within objects, so that's a no go. Also finding a key of an object pair with the value within the object is neigh impossible from what I can see (there's no way for us to say since id = 102, that 102 value's key is id and that id is within 'THUNDER'. The only maybe that I havent tried with this is to temporarily create a new object to compare the previous key to the new object? ie compare 'attributes' to a new object, and if it matches get the key from attributes using a through loop.

Or, we could just say screw it and go with a case statement. I mean, that would be the easier but uglier method. I'll post the code that I get working for that, and thennn you can decide if you like it, or want to keep battling it. lol

KHShadowrunner commented 8 years ago

Latest version of the code in feature/config addresses this. Maybe if we change the entirity of the ailments.json to the key = id, and have the obj contain both name and id again we could nuke the case statement. But the config file would be very interesting. Ill look into it

ThauEx commented 8 years ago

Can I close this issue?