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

Added several functions mainly related to banker NPC #3085

Merged
merged 4 commits into from
Jun 16, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jun 10, 2020

These functions were added in order for the upcoming sample banker NPC and in the future for the guild balance feature, I'd like to ask for a meticulous evaluation to avoid any error or exploit of the bank system. Please review function usefulness, names and possible enhancements

  • function getPlayerDatabaseInfo(name_or_guid)
    TFS 1.3 function that queries for player information from the database, returns a table of useful information if success, false if player doesn't exist.

getMoneyWeight(money)
getMoneyCount(string)
isValidMoney(money)
isNumber(str)
getPlayerDatabaseInfo(name_or_guid)
Player.depositMoney(self, amount)
Player.withdrawMoney(self, amount)
Player.canCarryMoney(self, amount)
Player.transferMoneyTo(self, target, amount)

@ghost ghost mentioned this pull request Jun 10, 2020
RomeroBA
RomeroBA previously approved these changes Jun 10, 2020
@Znote
Copy link
Member

Znote commented Jun 10, 2020

Could it be useful to have a general purpose "load player info from db" function? In order to reduce the amount of separate query requests to the SQL server?

Something along the lines of this:
https://gist.github.com/Znote/252757b013bd390e940cad65e8019fde

It returns player info (such as vocation, correct way to spell the name) upon success, false if player doesn't exist.

Znote added 2 commits June 15, 2020 03:29
Added function Player.canCarryMoney and updated existing code to work with getPlayerDatabaseInfo
Correct capacity conditional check
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.

Since nothing has been altered (just added content) this won't break anything.
I have actively tested these together with #2934

@Znote Znote merged commit 3cb5405 into otland:master Jun 16, 2020
@ghost ghost deleted the functionss branch June 16, 2020 12:24
@luanluciano93
Copy link
Contributor

Couldn't you directly use "return tonumber (money) ~ = nil and money> 0" instead of using the isValidMoney (money) function?

@RomeroBA
Copy link

Couldn't you directly use "return tonumber (money) ~ = nil and money> 0" instead of using the isValidMoney (money) function?

Yup, and he could also code everything into a single big ugly block of code, but creating functions for stuff that might be used later is a good idea.

@ghost
Copy link
Author

ghost commented Jun 25, 2020

Couldn't you directly use "return tonumber (money) ~ = nil and money> 0" instead of using the isValidMoney (money) function?

one check instead of two is fine 😋 i also thought about it but someone else said to keep it and so I did

@luanluciano93
Copy link
Contributor

I don't think two checks are good for something that can be done in one. To be honest, it shouldn't. use the compat.lua for nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants