Missing a few attributes in item description code

[Tofame]

Before creating an issue, please ensure:

• This is a bug in the software that resides in this repository, and not a
  support matter (use https://otland.net/forums/support.16/ for support)
• This issue is reproducible without changes to the C++ code in this repository

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]

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

[Tofame]

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]

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]

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.