Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Couple of Fixes #1222
Couple of Fixes #1222
Changes from 45 commits
e1e4d55
855dd7d
9017458
5e35e44
c9cfc6c
8753fc3
b584596
56f6e6c
ea20f76
a5d5714
646b9d1
4bd64cf
4934125
3be9ce6
c4ba839
980eb5b
1037519
229f4fa
c5bf09b
cc67d8c
422c945
841a230
0980cd8
422bacb
6df3fc4
0948503
ea8ac59
62b51d6
2dbe9ba
4240b0f
6bc4646
4a684b0
1786757
c52eea5
ceaf0a3
d7f7614
f7c948a
33db930
7b806e6
a36e879
592f0a6
e080570
0248026
3c8cfb8
ec132ce
642c031
e1859c1
000c555
787fbfd
262f4e4
dcaf0af
9a091a7
bebbf59
af32ecc
33dba6c
7d887da
3d86b36
4ad60ab
753ff28
b1ba663
82e59ee
75e5be9
5d92edc
6cf816b
665ded8
9951612
098b10e
2d0446f
3733da5
cc8ac5f
8c4d9e6
f00c638
b4b055f
07a466d
0d28677
9ccb383
2e0a8f5
9ff1647
688fd68
2f1d08f
0720adf
4e1a9c0
999f3af
0f7fc66
de007c2
a7a494a
cdc2e62
3b24fea
762b6c8
3e87445
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be useful then to be able to set a default cooldown in the ini? I admit i don't remember if that's already possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add default cooldown to ini for drinks, but it's different for ales and potions, but I can put some extra integers in sphere.ini for default cooldowns. But I add TDATA2 because people may want to keep default potion cooldowns but change some of them, like he may want to keep default cooldown for heal potion but change it for cure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel it would be wise to have a default cooldown for each type (maybe in the typedef? or ini?), then give the possibility to overwrite for the specific item. In the latter case, imho it would be better to store the info in a tag, so that it can be statically added to the typedef but also changed at runtime for maximum flexibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, could you please explain differently in the changelog what do the new locals do? I'm not sure i've grasped it fully
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I added TDATA2 exactly for this, it works well with @drink trigger, I mean, TDATA2 is something that you can set under [ITEMDEF] for any drink to define it's cooldown timer, but it can be override under @drink trigger dynamically, but if you still wish to add another tag that can be defined under [itemdef] and overridable dynamically, we can add TAG.OVERRIDE, https://wiki.spherecommunity.net/index.php?title=Override_TAGs like in here.
For LOCALS, they were for the POISONED WEAPONS, poison on weapon saved on morez, and it drops some value from it on every hit of player, normally this is 100%. but I added CHANCE and AMOUNT override to let gms decide how this poison effect removes, seems like I haven't put much info in changelog, I can improve it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay then for TDATA2 and @drink!
I'd only update the changelog entry for poisoned weapons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't this be added as a spellflag directly on the interested spell?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly it was the thing I thought at first, but there are other things affected from Elemental Engine, like defense calculation, RES DAM HITAREA etc, so I thought maybe we need to put some check for these props when they fully implemented, but if you think it's better to add spellflag for it I can revert this update back and add SPELLFLAG for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the elemental engine itself has different implications, but i'm not really convinced to add another variable (ElementalEngineFlags) in the ini just to configure single spells.
I'd see best a ini setting as something with a broad effect, or to config unscriptable stuff like network, muls etc. I fear to pollute the ini (which is already a big fat boy of a config file) with small tweaks affecting a single small thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personnally more option on the ini more cool it is. Important is to have good default configuration. This particular feature about elemental engine was asked by someone on discord. This fix make sense to me and give more possibility to personalise server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert Back the Elemental Engine update, and added SPELLFLAG_NO_ELEMENTALENGINE flag for spells.