1. Dismiss Notice
  2. GOG.com logo

    Thanks to YOUR votes, GOG.com now sells:
    - Sea Dogs - Sea Dogs: Caribbean Tales
    - Sea Dogs: City of Abandoned Ships

    Vote now to add Pirates of the Caribbean to the list!

    Dismiss Notice
  3. Under the Crossbones Podcast

    A Pirate Podcast with Interviews
    Music, Comedy and all things Pirate!

    - Episode Guide - About - Subscribe -
    - Twitter - Facebook - iTunes - Android -
    - Youtube - Fill the Coffers -

    Dismiss Notice
  4. New Horizons logo

    Quick links for PotC: New Horizons
    - Download latest version
    - Wiki - FAQ - Report bugs here
    - ModDB profile

  5. GOF logo

    Quick links for AoP2: Gentlemen of Fortune 2
    - Downloads and info
    - Historical Immersion Supermod
    - ModDB Profile

Dismiss Notice
New to the forum?
Please take a moment to read our Welcome Message and Forum Rules.

Fixed Bug with skill boost items that require multiple of the item to work

Discussion in 'Bug Archive' started by Tingyun, Aug 16, 2016.

  1. Levis

    Levis Find(Rum) = false; Staff Member Programmer Creative Support Storm Modder

    Joined:
    Oct 6, 2013
    Messages:
    6,679
    Gender:
    Male
    Occupation:
    ICT
    Location:
    University Twente (Netherlands)
    @Jack Rackham remember if you want to test this properly you need to start a new game each time because if you change something in the item array it wont change in game instantly... Altough F11 should fix it too I believe.

    @Tingyun could you test for me if the things work well (can't access my game atm).
    see what kind of bonusses you get with these items (I've give you some console commands to spawn the items)

    This should give a +2 in sailing exectute it twice to get two of those books and it should still give a +2
    Code:
    GiveItem2Character(pchar, "book4");
    
    This should give a +1 in commerce and a +1 in luck execute it again and it should still be +1
    Code:
    GiveItem2Character(pchar, "book3");
    
    This should give a +1 in cannons (after executing it 3 times) executing it 3 times again should make it +2 and it should go up untill +4 and not go higher.
    Code:
    GiveItem2Character(pchar, "mineral1");
    
    note if you give items trough the console it might be possible the mods aren't calculated instantly so you might need to move them in your inventory. altough I expect that not to be the case.
     
    Tingyun likes this.
  2. Pieter Boelen

    Pieter Boelen (Not So) Old Seadog Staff Member Administrator Storm Modder Hearts of Oak Donator

    Joined:
    Nov 11, 2004
    Messages:
    66,575
    Gender:
    Male
    Occupation:
    Maritime Research: Project Engineer (Analysis)
    Location:
    Wageningen, The Netherlands
    F11 should work fine to completely update the items on a savegame.
    There was a slight error with that before if you were carrying an Opium List, but that should be fixed now.

    The Give Item to Character function calls another function that does update the skill modifiers.
     
    Tingyun and Levis like this.
  3. Jack Rackham

    Jack Rackham HoO Team Member Quest Writer Storm Modder

    Joined:
    Jul 5, 2004
    Messages:
    3,287
    Gender:
    Male
    Occupation:
    math teacher
    Location:
    Sund, Ramsjö, Sweden
    Yes I have tested with a new game every time yes. And changed things until they work. :shrug

    I would appreciate if someone would test my file as well. I have run so many tests and couldn't find any
    bugs.
     
    Tingyun likes this.
  4. Tingyun

    Tingyun Corsair Storm Modder

    Joined:
    Aug 8, 2016
    Messages:
    1,445
    Gender:
    Male
    @Levis: As you requested I ran the console code you gave me, and watched the results. With Jack Rackham's fix everything looks good--the books only give their bonuses once, same with the pyrite (only gives a +1 max as soon as you have 3), and the pyrite and book bonuses never go away (I even executed the console 18 times, checking throughout, and no matter what all the skill bonuses were correct).
     
    Jack Rackham likes this.
  5. Levis

    Levis Find(Rum) = false; Staff Member Programmer Creative Support Storm Modder

    Joined:
    Oct 6, 2013
    Messages:
    6,679
    Gender:
    Male
    Occupation:
    ICT
    Location:
    University Twente (Netherlands)
    Pyrite should stack. so it should give +2 if you have 6 or more. Did it do this?
     
  6. Jack Rackham

    Jack Rackham HoO Team Member Quest Writer Storm Modder

    Joined:
    Jul 5, 2004
    Messages:
    3,287
    Gender:
    Male
    Occupation:
    math teacher
    Location:
    Sund, Ramsjö, Sweden
    Should it? Stack = 1 doesn't it mean >= 3 gives 1 bonus?
     
  7. Levis

    Levis Find(Rum) = false; Staff Member Programmer Creative Support Storm Modder

    Joined:
    Oct 6, 2013
    Messages:
    6,679
    Gender:
    Male
    Occupation:
    ICT
    Location:
    University Twente (Netherlands)
    That's what I tought first too. But according to @Pieter Boelen stack is just a true or false values (so 1 or 0) and if its 1 it should stack and if its 0 it shouldn't.
     
  8. Tingyun

    Tingyun Corsair Storm Modder

    Joined:
    Aug 8, 2016
    Messages:
    1,445
    Gender:
    Male
    Pyrite didn't stack in the test, bonus stayed at +1 throughout, even with 18 of them in inventory.
     
  9. Levis

    Levis Find(Rum) = false; Staff Member Programmer Creative Support Storm Modder

    Joined:
    Oct 6, 2013
    Messages:
    6,679
    Gender:
    Male
    Occupation:
    ICT
    Location:
    University Twente (Netherlands)
    Did you try my version also? it should work in there. or is something else going wrong there?
     
  10. Jack Rackham

    Jack Rackham HoO Team Member Quest Writer Storm Modder

    Joined:
    Jul 5, 2004
    Messages:
    3,287
    Gender:
    Male
    Occupation:
    math teacher
    Location:
    Sund, Ramsjö, Sweden
    But if stack = 1 means repeated bonus all items with stack = 1 should multilpy the bonus. That has not been the case
    in the game.

    For example 3 of jewelry2 (diamond) doesn't give triple bonus +3 leadership but +1 leadership.
    When using the original skill_utils.c.
     
  11. Levis

    Levis Find(Rum) = false; Staff Member Programmer Creative Support Storm Modder

    Joined:
    Oct 6, 2013
    Messages:
    6,679
    Gender:
    Male
    Occupation:
    ICT
    Location:
    University Twente (Netherlands)
    but if the stack says how often it can stack then stack 0 should always give a 0 skillbonus, so that's not logical either. That's why I agree with @Pieter Boelen that stack should be a bool value to say if the bonusses stack or not. And like I said we might need to look at some of the items to see if we do want them to stack or not....
     
  12. Tingyun

    Tingyun Corsair Storm Modder

    Joined:
    Aug 8, 2016
    Messages:
    1,445
    Gender:
    Male
    Levis: Your version did make it stack. I looked back and my test report for your file is on page 2 in the thread here: "Tested, no error log. But it makes the bonus stack. In other words, since 3 pyrite gives a +1 to the skill, 6 garnet now gives +2"

    For what it is worth, my personal opinion is that pyrite and other minerals should not stack the bonus (since you mention at looking whether you want them to stack). It is very easy to harvest native items, and stacking just seems too powerful. Better to cap it at +1, to avoid players getting +4 just through minerals.
     
  13. Levis

    Levis Find(Rum) = false; Staff Member Programmer Creative Support Storm Modder

    Joined:
    Oct 6, 2013
    Messages:
    6,679
    Gender:
    Male
    Occupation:
    ICT
    Location:
    University Twente (Netherlands)
    I agree it shouldn't stack, but thats a matter of changing it in the item. The code first needs to do what it is suppost to do. After that we can set the items right.
     
    Tingyun likes this.
  14. Tingyun

    Tingyun Corsair Storm Modder

    Joined:
    Aug 8, 2016
    Messages:
    1,445
    Gender:
    Male
    Oh, that makes sense. I just am ignorant of coding so I didn't understand at first.
     
  15. Jack Rackham

    Jack Rackham HoO Team Member Quest Writer Storm Modder

    Joined:
    Jul 5, 2004
    Messages:
    3,287
    Gender:
    Male
    Occupation:
    math teacher
    Location:
    Sund, Ramsjö, Sweden
    I still think my fix makes what it's supposed to do:

    It gives +1 leadership despite how many jewelry2 you've got.
    It gives +1 cannons from 3 mineral1 (pyrite) never more.
    It handles stack like the int it is. Test case: change mineral8 (+1 commerce) to stack 2 in InitItems.c. It now gives +2 commerce for each mineral8.
    (The case with both numneed & stack > 1 is not in the game and also limited by MAX_SKILL_INCREASE so it maybe should be removed.)

    to place in console:

    case "11":
    LAi_SetImmortal(pchar, true);
    RemoveCharacterEquip(Pchar, GUN_ITEM_TYPE);
    RemoveCharacterEquip(Pchar, BLADE_ITEM_TYPE);
    DeleteAttribute(pchar, "items");//takes all incl fists
    TakenItems(Pchar, "mineral1", 9); //pyrite: cannons+1
    TakenItems(Pchar, "mineral8", 2); //tormaline: commerce+1
    TakenItems(Pchar, "jewelry2", 3);
    break;

    This should give +1 cannons (from 9 pyrite)
    and +1 leadership from 3 dimonds
    and +4 commerce from 2 tormaline (if stack = 2)
     
  16. Levis

    Levis Find(Rum) = false; Staff Member Programmer Creative Support Storm Modder

    Joined:
    Oct 6, 2013
    Messages:
    6,679
    Gender:
    Male
    Occupation:
    ICT
    Location:
    University Twente (Netherlands)
    If the stack works as you say it works as it should. but if the stack works as @Pieter Boelen says your code does not work. That's my problem. I still can't find a definitive answer here what do we want stack to do?
    Should stack say how many of the bonus can be stacked?

    Also @Jack Rackham you should try your code with something which is stack 4, I think you will find it to react very weird then.
     
  17. Jack Rackham

    Jack Rackham HoO Team Member Quest Writer Storm Modder

    Joined:
    Jul 5, 2004
    Messages:
    3,287
    Gender:
    Male
    Occupation:
    math teacher
    Location:
    Sund, Ramsjö, Sweden
    Yes you're right it's wrong. :modding
    Already 1 item with numneed 1 and stack 4 gives a +4 bonus. More gives probably more but are limited by MAX_SKILL_INCREASE.

    PB:
    Or should if they gave -1 Luck.
    I agree with this explanantion.
     
  18. Jack Rackham

    Jack Rackham HoO Team Member Quest Writer Storm Modder

    Joined:
    Jul 5, 2004
    Messages:
    3,287
    Gender:
    Male
    Occupation:
    math teacher
    Location:
    Sund, Ramsjö, Sweden
    So were do we stand now? @Levis have you got anything that work or are you still
    working on it?
    Or do you want me to trying to fix my fix? With skill as an bool or not?
     
  19. Tingyun

    Tingyun Corsair Storm Modder

    Joined:
    Aug 8, 2016
    Messages:
    1,445
    Gender:
    Male
    EDIT: Deleted, realized I was wrong. ;)
     
    Last edited: Aug 19, 2016
  20. Pieter Boelen

    Pieter Boelen (Not So) Old Seadog Staff Member Administrator Storm Modder Hearts of Oak Donator

    Joined:
    Nov 11, 2004
    Messages:
    66,575
    Gender:
    Male
    Occupation:
    Maritime Research: Project Engineer (Analysis)
    Location:
    Wageningen, The Netherlands
    I'm pretty darn sure that "stack" is meant to be boolean that controls whether multiple of the same item give extra skills.
    It is not a multiplier as that would be even more massively overpowered.

    I can think of only one item that should stack: The Cursed Coin because it is evil.
    For any other items, I don't see the need. It is too easy to get your skills up through items as it is.

    Anyway, if you can't figure it out to everyone's satisfaction, just leave it as is until I get back.
    I can't be of any assistance whatsoever for the next few days, but I'll be around again eventually.
     
    Tingyun likes this.

Share This Page