Welcome to AC Web.
Results 1 to 8 of 8
  1. #1

    [C++] Item Upgrade


    REGISTER! (FREE)
    Registered members see less ads
    and also gain access to other great features.
    Hello AC-Web.

    Description
    The script enables an npc to upgrade items the user have equipped (for now at least) through gossip. An success rate can be set to the upgrade process. The upgradable item will not be consume on failure, however, the requirements will. The gossip has 4-5 menus.
    1. Main Menu
    2. Equipment slots.
    3. Recipes / Upgrade Information
    4. Upgrading Process / Remaining

    The reason behind this script was learning C++, therefore improvements, that is code related, are highly appreciated. Features are also welcomed.

    Details
    The system consists of two files. The script itself and a .sql file to the database with two tables. The .sql file contains test data with commands to add them for trying it out.
    When an item is upgradable it may have multiple recipes, which the system will show to the user (the recipe menu). If it only has one, it goes straight to the requirements.

    Download Files
    Last edited by Heitx; 08-25-2019 at 10:37 AM.

  2. #2
    weird like that

    Join Date
    Feb 2010
    Location
    http://rochet2.github.io/
    Posts
    5,268
    Looks very nice.

    Here are some notes after skimming through the code a bit:

    // global data to save information through gossip selection
    you should almost never store Player or Item pointers over time. Use ObjectGuid and player->GetGUID() instead. There is also a function to get an item by guid from the player.
    Consider for example that a player is just about to upgrade, but he deletes the items he uses for upgrade. The pointer will then be invalid as the item does not exist and a crash or worse can occur.
    If nothing else, fix at least this!

    player->ModifyMoney(-abs(upgrade.reqGold * 10000));
    Interesting choice for casting uint32 to signed.

    Something to know about auto is that for example
    auto recipes = depository[entry];
    will make a full deep copy of depository[entry] with all of the requirements and such.
    You may want to use auto& or delctype(auto) instead to make the type be reference instead. Same can be done in many of the loops.
    Adding const is also a nice extra to make sure you dont edit the static data in any way.

    PlayerData playerData = data[player];
    Seems you also make copies of PlayerData.

    uint itemEntry = item->GetTemplate()->ItemId;
    uint? what happened here?

    The PREFIX stings my eye a bit. You could just format it in it like everything else:
    .PSendSysMessage("%s[%s] -> [%s]", PREFIX, item->GetTemplate()->Name1, newItem->GetTemplate()->Name1);
    This might even remove a compiler warning about using dynamic format string.

  3. #3
    Quote Originally Posted by Rochet2 View Post
    Looks very nice.

    Here are some notes after skimming through the code a bit:

    // global data to save information through gossip selection
    you should almost never store Player or Item pointers over time. Use ObjectGuid and player->GetGUID() instead. There is also a function to get an item by guid from the player.
    Consider for example that a player is just about to upgrade, but he deletes the items he uses for upgrade. The pointer will then be invalid as the item does not exist and a crash or worse can occur.
    If nothing else, fix at least this!

    player->ModifyMoney(-abs(upgrade.reqGold * 10000));
    Interesting choice for casting uint32 to signed.

    Something to know about auto is that for example
    auto recipes = depository[entry];
    will make a full deep copy of depository[entry] with all of the requirements and such.
    You may want to use auto& or delctype(auto) instead to make the type be reference instead. Same can be done in many of the loops.
    Adding const is also a nice extra to make sure you dont edit the static data in any way.

    PlayerData playerData = data[player];
    Seems you also make copies of PlayerData.

    uint itemEntry = item->GetTemplate()->ItemId;
    uint? what happened here?

    The PREFIX stings my eye a bit. You could just format it in it like everything else:
    .PSendSysMessage("%s[%s] -> [%s]", PREFIX, item->GetTemplate()->Name1, newItem->GetTemplate()->Name1);
    This might even remove a compiler warning about using dynamic format string.
    At first look I saw some of these, I will look into the code more in-depth later. Otherwise code looks clean.

  4. #4
    Thanks for the code review. I have change everything you said (I hope) and make most of it constant. The script works flawlessly after the changes.

    I also tried to make the depository and data variables constant, however, that is not allowed since it makes them not modifiable (coming from Java universe, usually constant for objects means cannot be instantiated again).

  5. #5
    Another suggestion is to make the requirement item column to be a text so you can convert it to array so you can have unlimited item requirements in only 1 column :-)

  6. #6
    Quote Originally Posted by n1ghth2wk View Post
    Another suggestion is to make the requirement item column to be a text so you can convert it to array so you can have unlimited item requirements in only 1 column :-)
    That will require much more storage and might not be as beautiful. In addition, it destroys the 'third normal form'.

  7. #7
    hello friend all good..liked your item_upgrade script very much.
    but I need a change to your script
    could you do it for me i don't handle c ++
    and you can also pass it to module run ne azerothcore

  8. #8

    REGISTER! (FREE)
    Registered members see less ads
    and also gain access to other great features.
    seens well.
    I think item upgrade maybe has other way- upgrade item attributes,for example :
    an item has +100 intelligence
    use some item/money/or others and has chance to upgrade to +110 intelligence
    maybe cool!~
    If you have time, you can try this idea.(add a new script)

    in addition:
    now you use item_template.name,I think you should use localename,like rochet2 in transmog patch
    Last edited by qyhsandy; 09-03-2019 at 12:35 PM.

 

 

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •