otland / forgottenserver

A free and open-source MMORPG server emulator written in C++
https://otland.net
GNU General Public License v2.0
1.56k stars 1.04k forks source link

Missing a few attributes in item description code #4670

Open Tofame opened 2 months ago

Tofame commented 2 months ago

Before creating an issue, please ensure:

Steps to reproduce (include any configuration/script required to reproduce)

Its not technically a bug, but TFS's forgotten&unfinished feature

  1. Put: in any item in items.xml
  2. Equip this item
  3. It adds you 300 health, however the description of item wont say so.

Expected behaviour

Whole Item::getDescription from item.cpp, should be reworked. By that I mean divided into methods like,

Item::getDescription() {
   ... code
   addDescAbilityStats(s&)
   addDescIncrements(s&)
   addDescAbilityStatsPercents(s&)
   addDescReflects(s&)
   addDescAbsorbs(s&)
   ... code
}

The idea of maxhealthpoints and other stats like +X% of magic damage is TFS's custom thing. Tibia doesnt have it. But it is there (been for decades), so it should be fully finished, not only partly.

Also, if this comes through, the "protection" wording should also change. Tibia's style is listing protections in a way: protection earth +5%, holy +5%, earth -3% /\ all those are protections listed. But the +5% earth <- damage for example could be confusing in that case, so maybe each protection should be preceded by either "prot" or "protection" word.

Feel welcome to discuss. It's easy to finish and PR this, but Imo first it should be known what direction should this code go in.

EvilHero90 commented 2 months ago

Can you elaborate this more, I don't want to be rude but your text is heavily confusing, provide also some sample code

Tofame commented 2 months ago

Can you elaborate this more, I don't want to be rude but your text is heavily confusing, provide also some sample code

E.g. there is:

                       if (it.abilities->stats[STAT_MAGICPOINTS]) {
                if (begin) {
                    begin = false;
                    s << " (";
                } else {
                    s << ", ";
                }

                s << "magic level " << std::showpos << it.abilities->stats[STAT_MAGICPOINTS] << std::noshowpos;
            }

but at the same time a code that was omitted:

                        // MISSING ONES:
            if (it.abilities->stats[STAT_MAXHITPOINTS]) {
                s << ", max health " << std::showpos << it.abilities->stats[STAT_MAXHITPOINTS] << std::noshowpos;
            }
            if (it.abilities->stats[STAT_MAXMANAPOINTS]) {
                s << ", max health " << std::showpos << it.abilities->stats[STAT_MAXMANAPOINTS] << std::noshowpos;
            }

etc.

Then it would be good to remove those if's and put them into separate method: Pseudocode

addStatsToDesc(s&, it);

addStatsToDesc(s, it) {
    if (it.abilities->stats[STAT_MAGICPOINTS]) {
        if (begin) {
            begin = false;
            s << " (";
        } else {
            s << ", ";
        }

        s << "magic level " << std::showpos << it.abilities->stats[STAT_MAGICPOINTS] << std::noshowpos;
    }

    // MISSING ONES:
    if (it.abilities->stats[STAT_MAXHITPOINTS]) {
        s << ", max health " << std::showpos << it.abilities->stats[STAT_MAXHITPOINTS] << std::noshowpos;
    }
    if (it.abilities->stats[STAT_MAXMANAPOINTS]) {
        s << ", max health " << std::showpos << it.abilities->stats[STAT_MAXMANAPOINTS] << std::noshowpos;
    }
}

I think that some of abilities.stats and abilities.statsPercent are missing. Maybe some others too.

EvilHero90 commented 2 months ago

Yea I agree the monstrosity of that should have been re worked already. I might give this a shot once I'm done with my current PR's, I'll add it to the 1.6 milestone that we don't forget about it.

Tofame commented 2 months ago

I guess I was sent slightly older src, as currently tfs has moved that into lua.

So it doesn't really matter anymore. It's still 600+ lines method there, so it still could be broken into smaller segments (https://github.com/otland/forgottenserver/blob/master/data/lib/core/item.lua#L151), but atleast there are comments included.