Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing item attributes [$100] #998

Closed
tosty33 opened this issue Sep 30, 2014 · 14 comments · Fixed by #2807
Closed

Missing item attributes [$100] #998

tosty33 opened this issue Sep 30, 2014 · 14 comments · Fixed by #2807
Labels
bounty enhancement Increase or improvement in quality, value, or extent

Comments

@tosty33
Copy link
Contributor

tosty33 commented Sep 30, 2014

Missing Item Attributes

Hello,
There are some attributes that have been implemented in TFS 0.4, but that aren't added in 1.0.

Attribute list

Increasing power of melee / magic / healing:

increasemeleevalue
increasemeleepercent

increasemagicvalue
increasemagicpercent

increasehealingvalue
increasehealingpercent

Reflect system - something like absorb system, but the attacker gets some of the damage back:

reflectpercentall
reflectpercentelements
reflectpercentmagic
reflectpercentenergy
reflectpercentfire
reflectpercentearth
reflectpercentice
reflectpercentholy
reflectpercentdeath
reflectpercentlifedrain
reflectpercentmanadrain
reflectpercentdrown
reflectpercentphysical
reflectpercenthealing

For every reflect type:

reflectchanceall
reflectchanceelements
reflectchance...

Extra attributes for absorbing player / monster damage, could be useful for creating tank items:

absorbpercentmonsters
absorbpercentplayers

Also it should be possible to set those attributes by using item:setAttribute(...)

Bounty

Unfortunately I'm kinda busy, so I won't have time to work on it.
Fortunately Znote gave idea for using Bountysource, so I'm setting $65 bounty for adding those atributes.

There is a $100 open bounty on this issue. Add to the bounty at Bountysource.

@marksamman marksamman changed the title Missing item attributes Missing item attributes [$65] Sep 30, 2014
@Codinablack
Copy link
Contributor

Yes! Please. It would be so awesome to see how to add this into TFS 1.0 Because I need to do the same thing with items and with vocations. I would love to see this get done!

I also strongly believe that this will do the same thing as adding the cast system like znote was talking about, both of those added to this server and I can see MANY new developers switching distributions. Only thing that would be keeping people back from switching would be the doItemGetAttribute and doItemSetAttribute functions :D

@Kamenuvol
Copy link
Contributor

They should be categorized to avoid this long name.

<attribute type="reflection" element="fire" value="n"/>

or

<attrgroup type="reflection">
  <attribute type="fire" value = "n"/>
  <attribute type="ice" value = "m"/>
</attrgroup>

and the element should be customizable in other xml

<element type="fire" effect="n" condition="x"/>
<element type="music" effect="other"/><!--custom-->

So we can make custom absorb, reflection, etc

@Nu77
Copy link
Contributor

Nu77 commented Oct 1, 2014

Could be added attributes to handle health/mana too:

increaseHealthValue
increaseHealthPercent
increaseManaValue
increaseManaPercent

@tosty33
Copy link
Contributor Author

tosty33 commented Oct 1, 2014

@Nu77 That attributes are already in the engine:

maxhitpoints
maxhitpointspercent
maxmanapoints
maxmanapointspercent

@marksamman marksamman added the enhancement Increase or improvement in quality, value, or extent label Oct 1, 2014
@WibbenZ
Copy link
Member

WibbenZ commented Oct 20, 2014

@marksamman I have a code that I bought from Gesior, I could release it if he dosen't mind aswell.
But there is a problem with compiling on visual studio and he told me that it was something with that there was to many if / else if / else statments.
It compiles fine on ubuntu tho.
Is there a way to fix that? In that case I could put that code up here.

@tosty33
Copy link
Contributor Author

tosty33 commented Oct 20, 2014

@WibbenZ Yeah, visual studio has some problems when there are too many ifs / else ifs.
You can look here (search for _MSC_VER), it's a workaround for that VS error:
https://github.com/mattyx14/otxserver/blob/master/otxserver2/path_10x/sources/items.cpp

@marksamman marksamman changed the title Missing item attributes [$65] Missing item attributes [$100] Oct 30, 2014
@whitevo
Copy link

whitevo commented Sep 18, 2015

The above attributes on post 1 are quite irrelevant since we can manipulate them already in LUA trough creaturescripts.

What we can't do in LUA is change the container slot amount.

@WibbenZ
Copy link
Member

WibbenZ commented Sep 18, 2015

Correct but note the "manipulate" word.
To me that script seems like a hack insted of doing it hard coded.

@WibbenZ
Copy link
Member

WibbenZ commented Jan 30, 2016

@Crypton33, (Znote - can't tag him) and @marksamman
Im thinking about fixing this, but how do you guys want the XML code to look like?
Should we use the 0.x one (we will have to break the if statments down if we compile with visual studio, but we keep compatibility)
Or should we use a new one, that makes it look cleaner?
If you ask me we should do something new, since 1.x has never had item attributes so there kinda is no compatibility to retain.

This is what I did last night, looks cleaner to me atleast.

<attribute key="reflection" value="allelements">
    <attribute key="chance" value="100" />
    <attribute key="percent" value="100" />
</attribute>

<attribute key="reflection" value="energy">
    <attribute key="chance" value="100" />
    <attribute key="percent" value="100" />
</attribute>

It will take up more space, but as I said (to me) it looks alot cleaner compared to this:

reflectpercentelements
reflectpercentenergy

Still need to rewrite / add the other things you wrote about.
But these 2 below I can't find them in either (0.4) r3777 or r3884, are they even used or did you want them implemented?

increasemeleevalue
increasemeleepercent

@Znote
Copy link
Member

Znote commented May 31, 2016

@marksamman Do you have any guidelines which should be followed to help a developer create an acceptable pull request for this issue?

@marksamman
Copy link
Member

I'm not sure what the best way would be to implement this, and PR's with new features will have to wait until after the 1.2 release anyway. I'd rather see community effort around issues in the 1.2 milestone as the release is already long overdue.

@JSkalskiSBG
Copy link

I wrote these attributes/features for WibbenZ.
@WibbenZ
If you still have it, you can release it. You paid for it - it's your code now.

@marksamman
It's not possible to compile it in Visual Studio, because in "void Items::parseItemNode(const pugi::xml_node& itemNode, uint16_t id)":
https://github.com/otland/forgottenserver/blob/master/src/items.cpp#L539
are already 102 switch cases.
After adding all reflections and new attributes code reached over 128 switch cases. There is hard-coded limit of switch cases in VS C++ compiler. I read about it in some bug report to VS 2003 (yes, 2003!) and there were answers that they won't fix it. Even with many post from users that refreshed that bug report in 2013 and 2014, they decided to not change it.

// gesior at work

@ranisalt
Copy link
Member

ranisalt commented Oct 1, 2018

@JSkalskiSBG you could split the cases into several functions inside the cpp file - they will be inlined anyway.

@Zbizu
Copy link
Contributor

Zbizu commented Apr 23, 2020

is the problem with switch cases still occuring in 2017 and newer versions of visual studio?

EPuncker pushed a commit that referenced this issue Feb 3, 2022
from #998, for more details, refer to the PR #2807 itself
Codinablack pushed a commit to Codinablack/forgottenserver that referenced this issue Apr 5, 2022
from otland#998, for more details, refer to the PR otland#2807 itself

(cherry picked from commit a14f1fb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty enhancement Increase or improvement in quality, value, or extent
Projects
None yet
Development

Successfully merging a pull request may close this issue.