search

HerculesWS / Hercules

Hercules is a collaborative software development project revolving around the creation of a robust massively multiplayer online role playing game (MMORPG) server package. Written in C, the program is very versatile and provides NPCs, warps and modifications. The project is jointly managed by a group of volunteers located around the world as well as a tremendous community providing QA and support. Hercules is a continuation of the original Athena project.
http://herc.ws
GNU General Public License v3.0
903 stars 757 forks source link

Fix copying own skills via RG_PLAGIARISM or SC_REPRODUCE causing the skill to be deleted and skills that required it eventually. #3298

Closed skyleo closed 4 months ago

skyleo commented 6 months ago

Pull Request Prelude

Changes Proposed

We address this by implementing logic to use SKILL_FLAG_REPLACED_LV_0 in SC_REPRODUCE and RG_PLAGIARISM. We also reduced some code-duplication shared with pc_jobchange and skill_attack logic of those two skills. SKILL_FLAG_PERM_GRANTED skills are no longer copyable as there's no clearly defined implementation for those when copied.

Note: A skill that one has the prerequisites for but one didn't level will appear as SKILL_FLAG_PERMANENT and have it's .id entry set in an sd's status.skill[] entry. Note: Well skills that you didn't learn, nor can learn, will also appear as SKILL_FLAG_PERMANENT, but can be distinguished via unset id.

Vaguely inspired by https://github.com/HeraclesHub/Heracles/pull/14

Issues addressed: #3289

List of commits in my private-fork that this PR is based on:

dfbdbc0335fd05e5317f1efbefb16cac3b7a9050
6e29dc9c1e92453edfe5930d154687200a1e658f
87ba9544695c4e4d332c2f24504d0cb76a6161a3
9d15215176a525ed3aabfcfdbb89ba877b94c60c
a6ecba8539ab3178d8748c098e27bf808e0420cf
a4bd3b1240274da0650cfa2832a82fa2f15f2c3e
4cea95b2391740e9a3f002ac56d0132608bd3f9c
b82d6ca0d7e890a6d25271c7ecf4081d4654e531
skyleo commented 6 months ago

I think there is some missing packet (at least for "newer" clients), on 2019-12-24 main, when a new skill is copied by Plagiarism (Rogue one) the old one stays in the tree as "unlearned":

Did you test this on a rogue / stalker and a character that doesn't own skills outside of his skill tree (such as GMs with all skills)

Cause this new logic should not touch these skills that you copied unless you are using this artificial case o-o

guilherme-gm commented 6 months ago

Hmm I did use @allskills (using default herc configs, where only my class skill tree gets filled -- so no extra skill in my character).

My test was more or less like that:

then, on a pvp map, use another character to hit me with Magnum Break, which got copied. Then hit by Bash, which got copied but did not erase Magnum Break there.

skyleo commented 6 months ago

Hmm I did use @allskills (using default herc configs, where only my class skill tree gets filled -- so no extra skill in my character).

My test was more or less like that:

* `@job Shadow Chaser T.`

* `@allskills`

then, on a pvp map, use another character to hit me with Magnum Break, which got copied. Then hit by Bash, which got copied but did not erase Magnum Break there.

Please don't test stuff like this with @allskills, try to test it on a somewhat realistic character. @job and @jlvl should be enough.

If you can reproduce that way I'll try to research.

skyleo commented 5 months ago

Problem discovered, fixed and accordingly rebased the commits. Introducing a new function pc->is_own_skill now as first commit.