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

[WIP] Store System #2143

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions config.lua
Expand Up @@ -29,6 +29,10 @@ statusTimeout = 5000
replaceKickOnLogin = true
maxPacketsPerSecond = 25

-- Store configuration
storeImagesUrl = "http://otserv.info/images/store/"
storeCoinsPacketSize = 25

-- Deaths
-- NOTE: Leave deathLosePercent as -1 if you want to use the default
-- death penalty formula. For the old formula, set it to 10. For
Expand Down
2 changes: 1 addition & 1 deletion data/events/events.xml
Expand Up @@ -16,7 +16,7 @@
<event class="Player" method="onLookInBattleList" enabled="1" />
<event class="Player" method="onLookInTrade" enabled="1" />
<event class="Player" method="onLookInShop" enabled="0" />
<event class="Player" method="onMoveItem" enabled="0" />
<event class="Player" method="onMoveItem" enabled="1" />
<event class="Player" method="onMoveCreature" enabled="0" />
<event class="Player" method="onReport" enabled="1" />
<event class="Player" method="onTurn" enabled="0" />
Expand Down
19 changes: 19 additions & 0 deletions data/events/scripts/player.lua
Expand Up @@ -86,6 +86,25 @@ function Player:onLookInShop(itemType, count)
end

function Player:onMoveItem(item, count, fromPosition, toPosition, fromCylinder, toCylinder)
-- Store Inbox
Copy link
Contributor

Choose a reason for hiding this comment

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

That here will not work for containers inside the store inbox.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really, I didn't test this.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is also the problem of equipping the item, lets say you ahve a shield in your hand and you buy another shield in the store, if you move the shield in the store inbox it will replace with your equipped one and that shouldn't happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reality is that I took advantage this script from an otland post and did not do all the testing if you have something ready to share with us, I'll appreciate it. 😱

Copy link
Member

@dbjorkholm dbjorkholm Feb 3, 2017

Choose a reason for hiding this comment

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

I would probably opt for a source modification here instead of relying on 'onMoveItem'. An example would be to create a new class called 'StoreInbox' which mimics the behavior of the normal Inbox. It wouldn't let the player to put item(s) inside the store inbox, nor its child containers, and even switching shields wouldn't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, good idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

that is not the purpose of lua player events, it's a matter of customization, your system should not have any dependency on Lua api

Copy link
Member

Choose a reason for hiding this comment

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

Wrong, events was created so we had a place to put the "old" C++ code when we moved it.
Idk why ppl seem to think that we should have everything in C++ when we can place it in Lua.

So IMO nothing wrong there.

Copy link
Contributor

Choose a reason for hiding this comment

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

@WibbenZ server working stuff should stay in binaries and left lua as a easily customization/server setup or changing the default behaviour, never server working stuff, datapack are supossed to be an example for people build their own datapack, which means server should not have dependency on datapack scripts, otherwise datapack is not an example for a datapack but a server working stuff, and it force people to use default tfs datapack.

Copy link
Member

Choose a reason for hiding this comment

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

I get what you are saying but we should keep as much in Lua as posible.
Having some codes in Lua can be faster than having them in c++.

Another reason for that is so we can modify things without needing to recompile everytime.

And gl using a 1.1 datapack with 1.3 or even 1.2 without doing changes.
Thats a thing we need to do, either as new prs are approved or once every year or so.

Either update your server files or stay where you are.

Copy link
Contributor

Choose a reason for hiding this comment

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

we should keep as much in Lua as posible.

i don't think so, imo we should increase lua coverage over server default behaviour, not move server to lua, it would be a disaster, we really shouldn't

Having some codes in Lua can be faster than having them in c++.

even once in a blue moon cases, isn't a matter of server performance

And gl using a 1.1 datapack with 1.3 or even 1.2 without doing changes.

that's why wiki provide server version for lua implementation, you're not suppose to use datapack 1.1 on 1.2, but any 1.2 datapack should be fully compatible with 1.2 server

Copy link
Contributor

Choose a reason for hiding this comment

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

Tfs is a server I hope you know that? ;)
wtf? reason why i wrote that is that you said ANY SERVER (any server include tfs, or dota server and whos know how many more) so gl breaking valve's servers.

i think you still not get the point, the point of my comments, i'm repeating myself, but ok look, you agree that player walking event shouldn't stop working because of bad stepin/stepout event? so why store inbox should?

Copy link
Member

Choose a reason for hiding this comment

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

I semi agree with you, as I have said, if a movement script bugged that the player now uses it should bug.
If there is a problem with the store inbox it should bug.

Having 50% of a logic working dosnt seem like a good ide to me.
But it might be better if we ask @djarek and @ranisalt

Copy link
Member

Choose a reason for hiding this comment

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

@wgrriffel not pending to any side, but comparing to Dota is too shallow here. TFS IS meant to be MUCH more flexible, as it does not accept any user custom script as Dota does (as you mentioned, I don't play it), so all scripts are to be validated by the server owner.

That said, I'd like more of the core moved to Lua even though it's easily "crashable" by a bad scripter. Store inbox being a Lua event would be easy to e.g. notify a player if something happens, or add more/remove some checks. To me, it's a very valid tradeoff.

You guys should open a new issue to discuss that in a more generic way, this review is getting way too large and hard to follow.

Copy link
Contributor

@wgrr wgrr Feb 3, 2017

Choose a reason for hiding this comment

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

@ranisalt you're mistaken, it's a fair comparation, valve distribute their server same as tfs, you can configure it to have your own server, ofc they don't provide source code for engine, which means you can have a dota server up running and connect throught official client, just like counter-strike 1.6 if you remember, then user has to validade their scripts as well, my point was they trust pretty much in their lua api, they provide their host with runnign server to your scripting, that what i meant, anyway it gonne too off topic now, the problem never was "crashable" but without onMoveItem event the store will not work properly, it's my problem with that. tho im ok with a new event to onMoveStoreInbox, as i said before user should be able to change default behaviour, whats the point of engine rely on scripting? that's awful.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wanna also point out the event Player:onLook, this same discussion could be applied to it but in the end it was moved to Lua so I think this should be in Lua too.

local containerIdFrom = fromPosition.y - 64
local containerFrom = self:getContainerById(containerIdFrom)
if containerFrom then
if containerFrom:getId() == ITEM_STORE_INBOX and toPosition.y >= 1 and toPosition.y <= 11 and toPosition.y ~= 3 then
self:sendCancelMessage(RETURNVALUE_CONTAINERNOTENOUGHROOM)
return false
end
end

local containerTo = self:getContainerById(toPosition.y - 64)
if containerTo then
if (containerTo:getId() == ITEM_STORE_INBOX) then
self:sendCancelMessage(RETURNVALUE_CONTAINERNOTENOUGHROOM)
return false
end
end
-- End Store Inbox

return true
end

Expand Down
7 changes: 7 additions & 0 deletions data/items/items.xml
Expand Up @@ -31181,6 +31181,13 @@
<attribute key="weight" value="2504" />
<attribute key="description" value="Only the real killers can touch me!" />
</item>
<item id="24770" article="a" name="Store coin">
<attribute key="weight" value="0" />
<attribute key="description" value="Store Coins are needed to shop exclusive products in the Store, including fabulous mounts, classy outfits, a Name Change or even Premium Time." />
</item>
<item id="26052" name="your store inbox">
<attribute key="containerSize" value="20" />
</item>
<item id="30001" name="water" />
<item id="30002" name="blood" />
<item id="30003" name="beer" />
Expand Down
8 changes: 7 additions & 1 deletion data/migrations/19.lua
@@ -1,3 +1,9 @@
function onUpdateDatabase()
return false
print("> Updating database to version 19 (store system)")

db.query("ALTER TABLE `accounts` ADD COLUMN `coins` int(11) NOT NULL DEFAULT '0'")

db.query("CREATE TABLE IF NOT EXISTS `store_history` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `account_id` int(11) NOT NULL, `coins` int(10) NOT NULL DEFAULT '0', `description` varchar(256) NOT NULL DEFAULT '', `timestamp` bigint(20) unsigned NOT NULL, PRIMARY KEY (`id`), FOREIGN KEY (`account_id`) REFERENCES `accounts`(`id`) ON DELETE CASCADE ) ENGINE=InnoDB;")

return true
end
3 changes: 3 additions & 0 deletions data/migrations/20.lua
@@ -0,0 +1,3 @@
function onUpdateDatabase()
return false
end
36 changes: 36 additions & 0 deletions data/store/scripts/blessings.lua
@@ -0,0 +1,36 @@
local blessings = {
['the wisdom of solitude'] = 1,
['the spark of the phoenix'] = 2,
['the fire of the suns'] = 3,
['the spiritual shielding'] = 4,
['the embrace of server'] = 5,
['twist of fate'] = 6
}

local function getAffectedBlessings(offerName)
if offerName == 'all regular blessings' then
return {1, 2, 3, 4, 5}
end

return {blessings[offerName]}
end

function onRender(player, offer)
local checkBlessings = getAffectedBlessings(offer:getName():lower())
for _, blessing in ipairs(checkBlessings) do
if player:hasBlessing(blessing) then
return false
end
end

return true
end

function onBuy(player, offer)
local affectedBlessings = getAffectedBlessings(offer:getName():lower())
for _, blessing in ipairs(affectedBlessings) do
player:addBlessing(blessing)
end

return true
end
21 changes: 21 additions & 0 deletions data/store/scripts/items.lua
@@ -0,0 +1,21 @@
function onRender(player, offer)
local itemType = ItemType(offer:getName())
if itemType:getId() == 0 then
return false, "Item not found. Please contact an administrator."
end

return true
end

function onBuy(player, offer)
local itemType = ItemType(offer:getName())
local inbox = player:getSlotItem(CONST_SLOT_STORE_INBOX)
if inbox and inbox:getEmptySlots() >= 1 then
inbox:addItem(itemType:getId(), 1)
else
player:sendStoreError(STORE_ERROR_PURCHASE, "Please make sure you have free slots in your store inbox.")
return false
end

return true
end
15 changes: 15 additions & 0 deletions data/store/scripts/mounts.lua
@@ -0,0 +1,15 @@
function onRender(player, offer)
local enabled = not player:hasMount(offer:getName())
local reason = enabled and "" or "You already have " .. offer:getName() .. " Mount."
return enabled, reason
end

function onBuy(player, offer)
local success = player:addMount(offer:getName())
if not success then
player:sendStoreError(STORE_ERROR_PURCHASE, "Mount '" .. offer:getName() .. "' was not found. Please contact the administrator.")
return false
end

return true
end
18 changes: 18 additions & 0 deletions data/store/scripts/namechange.lua
@@ -0,0 +1,18 @@
function onRender(player, offer)
return true
end

function onBuy(player, offer, param)
local result = db.storeQuery("SELECT `id` FROM `players` WHERE `name` = " .. db.escapeString(param))
if result then
player:sendStoreError(STORE_ERROR_PURCHASE, "The name '" .. param .. "' is already being used.")
return false
end

if not db.query("UPDATE `players` SET `name` = " .. db.escapeString(param) .. " WHERE `id` = " .. player:getGuid()) then
player:sendStoreError(STORE_ERROR_PURCHASE, "Something went wrong, please try again later.")
return false
end

return true
end
76 changes: 76 additions & 0 deletions data/store/scripts/outfits.lua
@@ -0,0 +1,76 @@
-- Load outfits
local outfits = {}

for line in io.lines('data/XML/outfits.xml') do
Copy link
Contributor

Choose a reason for hiding this comment

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

This is ugly, it would be better to have a Game.getOutfits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😓
Help me with this, I'm sharing my time at work to do some contributions, is very little time :(

Copy link
Member

Choose a reason for hiding this comment

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

Or even better an Outfit class so we don't have to send huge tables when we might just want the sex of a citizen outfit.

local sex = tonumber(line:match('type.-=[\'"](%d-)[\'"]'))
local lookType = tonumber(line:match('looktype.-=[\'"](.-)[\'"]'))
local name = line:match('name.-=[\'"](.-)[\'"]')
local enabled = line:match('enabled.-=[\'"](.-)[\'"]') == 'yes'
local unlocked = line:match('unlocked.-=[\'"](.-)[\'"]') == 'yes'
Copy link
Member

Choose a reason for hiding this comment

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

Regexing a XML file: please DON'T YOU EVER DARE.

Here's why.


if enabled then
local outfit = outfits[name:lower()]
if not outfit then
outfit = {}
outfits[name:lower()] = outfit
end
table.insert(outfit, lookType)
outfit.unlocked = unlocked
end
end

local function getAffectedOutfit(offerName)
local outfitName
local addons
if offerName:sub(1, 5) == 'full ' then
outfitName = offerName:match('^full (.-) outfit$')
addons = {1, 2}
else
outfitName = offerName:match('^(.+) outfit')
addons = {tonumber(offerName:match('outfit addon (.-)$'))}
end

return outfits[outfitName], addons
end

function onRender(player, offer)
local lookTypes, addons = getAffectedOutfit(offer:getName():lower())

if not lookTypes then
return false, 'Outfit "' .. offer:getName() .. '" was not found. Please contact the administrator.'
end

for _, lookType in ipairs(lookTypes) do
if #addons == 0 then
if player:hasOutfit(lookType, 0) or lookTypes.unlocked then
return false, "You already have " .. offer:getName() .. " Outfit."
end
elseif #addons == 1 then
local enabled = (player:hasOutfit(lookType, 0) or lookTypes.unlocked) and not player:hasOutfit(lookType, addons[1])
local reason = enabled and "" or "You already have this addon."
return enabled, reason
else
for _, addon in ipairs(addons) do
if player:hasOutfit(lookType, addon) then
return false, "You already have one these addons."
end
end
end
end

return true
end

function onBuy(player, offer)
local lookTypes, addons = getAffectedOutfit(offer:getName():lower())

for _, lookType in ipairs(lookTypes) do
player:addOutfit(lookType)

for _, addon in ipairs(addons) do
player:addOutfitAddon(lookType, addon)
end
end

return true
end
14 changes: 14 additions & 0 deletions data/store/scripts/premiumtime.lua
@@ -0,0 +1,14 @@
function onRender(player, offer)
return true
end

function onBuy(player, offer)
local days = tonumber(offer:getName():match('(%d+) days$'))
if days then
player:addPremiumDays(days)
return true
end

player:sendStoreError(STORE_ERROR_PURCHASE, 'Offer "' .. offer:getName() .. '" not found. Please contact the administrator.')
return false
end
13 changes: 13 additions & 0 deletions data/store/scripts/sexchange.lua
@@ -0,0 +1,13 @@
function onRender(player, offer)
return true
end

function onBuy(player, offer)
player:setSex(player:getSex() == PLAYERSEX_FEMALE and PLAYERSEX_MALE or PLAYERSEX_FEMALE)

local playerOutfit = player:getOutfit()
playerOutfit.lookType = player:getSex() == PLAYERSEX_FEMALE and 137 or 129
player:setOutfit(playerOutfit)

return true
end
22 changes: 22 additions & 0 deletions data/store/scripts/templeteleport.lua
@@ -0,0 +1,22 @@
local MIN_DISTANCE = 50 -- Distance in SQM's

function onRender(player, offer)
if player:isPzLocked() then
return false, "You can not purchase a transportation service while PZ Locked."
end

if Tile(player:getPosition()):hasFlag(TILESTATE_NOLOGOUT) then
return false, "You can not purchase a transportation service while in a no-logout zone."
end

if player:getPosition():getDistance(player:getTown():getTemplePosition()) < MIN_DISTANCE then
return false, "You can not purchase this transportation service this close to your temple."
end

return true
end

function onBuy(player, offer)
player:teleportTo(player:getTown():getTemplePosition())
return true
end