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.

Sign up for GitHub

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

Jump to bottom

Added Loyalty System (10.74) #4148

Closed
wants to merge 5 commits into from
Closed

Added Loyalty System (10.74) #4148

wants to merge 5 commits into from

Conversation

reyaleman
Copy link
Contributor

Pull Request Prelude

Changes Proposed

  • Implemented LOYALTY SYSTEM
  • System is optional, can be disabled in config.lua (loyaltyEnabled = false)
  • Loyalty bonuses are customizables in config.lua
  • Modified Player::GetPercentLevel to return floats (client support decimal numbers on skill bars)
  • Loyalty Titles onLook
  • Loyalty message bonus on serverlog (Player:onLogin script)
  • Kept skill & tries untouched, loyalty is calculated when the player is loaded.
  • Loyalty points are updated on startup but only if it's been more than 1 day since last update (server config)
  • Added 3 lua functions (Player:getLoyaltyStatus, Player:addLoyalty & Player:removeLoyalty)
  • Added migration file to update database (column "loyalty_points" added to accounts table and inserted server config "loyalty_update" value to update loyalty on a daily basis)

Missing:

  • Social loyalty badges (social not implemented yet)
  • Loyalty highscores (highscores not implemented yet)

loyalty

Issues addressed:
#3871 Version 10.74 (The Loyalty System was introduced.)

Any comments would be highly appreciated. 😃

@ghost ghost requested review from DSpeichert and a team May 30, 2022 02:29
DSpeichert
DSpeichert requested changes May 30, 2022
View reviewed changes
schema.sql Outdated Show resolved Hide resolved
omarcopires
omarcopires reviewed May 30, 2022
View reviewed changes
src/player.cpp Outdated Show resolved Hide resolved
nekiro
nekiro requested changes May 31, 2022
View reviewed changes
Copy link
Member

@nekiro nekiro left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great stuff

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

bool Player::addLoyalty(int32_t points)
{
if (loyaltyPoints == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

I might not understand how this work, but why would you not allow adding loyalty points if loyaltyPoints is 0?

Copy link
Contributor Author

@reyaleman reyaleman May 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

yep, my bad... I was using this as a "loyaltyEnabled" check... should I add a check if LOYALTY SYSTEM is enabled or allow users to give loyalty points even if the system is disabled?

Copy link
Member

@nekiro nekiro May 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure, but I think it doesn't matter, so just let it work even with loyalty system disabled

@ghost ghost added the feature New feature or functionality label Jun 2, 2022
@ghost ghost requested review from ranisalt and ramon-bernardo June 2, 2022 07:43
pr comments + fixes
dbc1457 
fixed an issue when advancing base skills/maglevel not syncing with bonus "loyalty" skills... now it's working as expected (cipbia)
nekiro
nekiro reviewed Jun 2, 2022
View reviewed changes
src/configmanager.cpp
@@ -334,6 +364,19 @@ float ConfigManager::getExperienceStage(uint32_t level) const
return std::get<2>(*it);
}

float ConfigManager::getLoyaltyBonus(uint16_t points) const
{
auto it = std::find_if(loyaltyBonuses.begin(), loyaltyBonuses.end(), [points](LoyaltyBonuses::value_type bonus) {
Copy link
Member

Choose a reason for hiding this comment

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

auto it = std::find_if(loyaltyBonuses.begin(), loyaltyBonuses.end(), [points](const auto& bonus) {?

Copy link
Member

Choose a reason for hiding this comment

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

It should just be (auto bonus), otherwise 👍

src/luascript.cpp

uint16_t points = getNumber<uint16_t>(L, 2, 1);
pushBoolean(L, player->addLoyalty(-points));

Copy link
Member

Choose a reason for hiding this comment

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

remove newline

src/luascript.cpp

uint16_t points = getNumber<uint16_t>(L, 2, 1);
pushBoolean(L, player->addLoyalty(points));

Copy link
Member

Choose a reason for hiding this comment

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

remove newline

src/player.cpp
@@ -146,6 +151,16 @@ std::string Player::getDescription(int32_t lookDistance) const
} else {
s << " has no vocation.";
}

if (g_config.getBoolean(ConfigManager::LOYALTY_SYSTEM) && loyaltyPoints >= 50) {
if (sex == PLAYERSEX_FEMALE) {
Copy link
Member

Choose a reason for hiding this comment

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

use fmt::format?

src/player.cpp
@@ -126,6 +126,11 @@ std::string Player::getDescription(int32_t lookDistance) const
} else {
s << " You have no vocation.";
}

if (g_config.getBoolean(ConfigManager::LOYALTY_SYSTEM) && loyaltyPoints >= 50) {
s << " You are " << getLoyaltyTitleDescription(loyaltyPoints) << " "
Copy link
Member

Choose a reason for hiding this comment

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

use fmt::format?

src/player.cpp

count = static_cast<uint64_t>(amount);

if (manaSpentFractional == (int)manaSpentFractional) {
Copy link
Member

Choose a reason for hiding this comment

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

use static_cast and int32_t

@ghost ghost requested review from DSpeichert and nekiro June 9, 2023 14:51
ranisalt
ranisalt reviewed Jun 9, 2023
View reviewed changes
config.lua.dist
@@ -135,6 +135,22 @@ experienceStages = {
{ minlevel = 101, multiplier = 3 }
}

-- Loyalty system
loyaltyEnabled = true
Copy link
Member

Choose a reason for hiding this comment

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

You can drop this flag and let users effectively disable the loyalty system by removing all tiers.

@omarcopires
Copy link
Contributor

could you leave some extra functions in LUA to be used in cyclopedia?
getLoyaltyMagicLevel, getLoyaltySkill and getLoyaltyTitleDescription

I also think it would be better to move the descriptions to onLook instead of keeping them directly in the engine

@reyaleman
Copy link
Contributor Author

I'd have to rewrite this and make a new pull request (taking your comments in consideration), IIRC I stopped this because I was having issues with float values being inexact 👀

@reyaleman reyaleman closed this Mar 14, 2024
@reyaleman reyaleman deleted the account-loyalty branch March 14, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omarcopires omarcopires omarcopires left review comments

@ranisalt ranisalt ranisalt left review comments

@ramon-bernardo ramon-bernardo Awaiting requested review from ramon-bernardo

@DSpeichert DSpeichert Awaiting requested review from DSpeichert

@nekiro nekiro Awaiting requested review from nekiro

Assignees
No one assigned
Labels
feature New feature or functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@reyaleman @omarcopires @DSpeichert @ranisalt @nekiro