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

Skull system #3385

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open

Skull system #3385

wants to merge 31 commits into from

Conversation

rookgaard
Copy link
Contributor

Pull Request Prelude

Changes Proposed

Introduced skull system described in https://tibia.fandom.com/wiki/Skull_System

@rookgaard rookgaard changed the title Feature/skull system Skull system Mar 25, 2021
@rookgaard
Copy link
Contributor Author

@EPuncker Can you add reviewers?

@EPuncker EPuncker requested a review from a team March 25, 2021 23:49
src/player.cpp Outdated Show resolved Hide resolved
src/iologindata.cpp Outdated Show resolved Hide resolved
src/iologindata.cpp Outdated Show resolved Hide resolved
src/player.cpp Outdated Show resolved Hide resolved
src/player.cpp Outdated Show resolved Hide resolved
src/player.cpp Outdated Show resolved Hide resolved
src/player.cpp Outdated Show resolved Hide resolved
@rookgaard rookgaard requested a review from nekiro March 26, 2021 00:16
@MillhioreBT
Copy link
Contributor

Please use the new string formatting style fmt

@rookgaard
Copy link
Contributor Author

@MillhioreBT Where do you want me to use that formatting?

@MillhioreBT
Copy link
Contributor

@MillhioreBT Where do you want me to use that formatting?

Sorry for not specifically suggesting where
In everything that makes use of: std::ostringstream

This is suggested in the PR #3351, apparently the idea is to get rid of the use of std::ostringstream and using fmt instead

@rookgaard
Copy link
Contributor Author

rookgaard commented Mar 26, 2021

@MillhioreBT In my opinion, as long as there are usages of ostringstream inside that file, we shouldn't mix up different approaches. There should be a separate PR to replace all uses to fmt in iologindata.cpp.

src/player.cpp Outdated Show resolved Hide resolved
src/player.cpp Outdated Show resolved Hide resolved
src/iologindata.cpp Outdated Show resolved Hide resolved
src/iologindata.h Outdated Show resolved Hide resolved
@rookgaard rookgaard requested a review from ranisalt March 26, 2021 12:23
src/player.cpp Outdated Show resolved Hide resolved
src/player.cpp Outdated Show resolved Hide resolved
@rookgaard rookgaard requested a review from nekiro March 26, 2021 13:47
Copy link
Member

@ranisalt ranisalt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather have the control be done in Lua instead, but I consider it an improvement over the current situation.

@MillhioreBT
Copy link
Contributor

I would rather have the control be done in Lua instead, but I consider it an improvement over the current situation.

I also support the idea of ​​handling this in lua, as this seems to be a feature that anyone would want to manipulate for their own purposes and modifications ;), but as rani says, this is fine in any case

@nekiro
Copy link
Member

nekiro commented Mar 28, 2021

It actually shouldn't be hard to move to lua, it's bunch of queries and some logic and we have ondeath/onpreparedeath/onkill events, so you can slap skull system there easily. Honestly I don't see any limitations to make this in lua.

@EPuncker EPuncker requested a review from ranisalt April 19, 2021 17:55
EPuncker
EPuncker previously approved these changes Apr 19, 2021
Copy link
Contributor

@EPuncker EPuncker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets leave the Lua part to another PR shall we? 😛 I'm in favor of this change as it is an improvement of the current one

@rookgaard
Copy link
Contributor Author

@nekiro @DSpeichert

@EPuncker EPuncker requested a review from a team May 12, 2021 19:19
Copy link
Member

@Znote Znote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must admit I like the OT skull system more, I have had lots of fun on some custom rpg styled pvp servers that had like a constant 2-4 hour to loose 1 frag, without any daily/weekly/monthly limits. As it opens up for the possibility to kill lots of people daily without getting red/black skull, while still preventing "mass pk incidents" where someone would slay all lowbies in the area.

So I think you should re-implement the previous behavior as an optional toggle in config that just uses timeToDecreaseFrags. Retain "backwards compatibility" with skull system, perhaps we can call it oldOTSkullSystem = bool
Instead of fixed red/black skull duration, make the duration be based on remaining frags (when you have 0 frags, you loose red/black skull).

If I have 2 hour to decrease frag, how would I do it with this new skull system? Go 24 hour (daily limit) / 2 = 12 frags to red skull, set weekly and monthly to 9000+ to avoid those calculations. The risk is here that someone will spam UE and kill 12 people right off the bat, whereas with the retro OT skull system they had to "space out" their killing spree throughout the day. (Since they would get red skull after 3 kills, but are relatively quickly loosing frags).

@EPuncker EPuncker dismissed stale reviews from DSpeichert, omarcopires, and themself via 44c166c May 8, 2022 01:28
EPuncker
EPuncker previously approved these changes May 8, 2022
@EPuncker EPuncker requested a review from DSpeichert May 8, 2022 01:36
@EPuncker
Copy link
Contributor

EPuncker commented May 8, 2022

@DSpeichert I rebased it and solved conflicts.

ramon-bernardo
ramon-bernardo previously approved these changes Nov 2, 2022
src/player.cpp Outdated
@@ -4039,17 +4039,49 @@ void Player::addUnjustifiedDead(const Player* attacked)

sendTextMessage(MESSAGE_EVENT_ADVANCE, "Warning! The murder of " + attacked->getName() + " was not justified.");

skullTicks += g_config.getNumber(ConfigManager::FRAG_TIME);
Skulls_t playerSkull = getSkull();
if (playerSkull == SKULL_ORANGE) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far i understand this orange skull check here does not make sense. The orange skull must be checked on the attacked player (the player which has been killed) and its a "client skull" (a skull that only appear to you at certain conditions like yellow skull, party skull, warmode skull etc) manipulated by the Creature::getSkullClient function.

@pasturryx
Copy link

when merged is going to be added?

@ranisalt
Copy link
Member

I believe this system can be implemented in Lua. When a player is killed by another, recalculate the skull; using a storage value to keep track of skull duration and an event to clear it if the player is online when it expires, or check on login. We already have skull functions in Lua.

@omarcopires omarcopires mentioned this pull request Dec 20, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Increase or improvement in quality, value, or extent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet