Skip to content

blessings updated#4479

Open
reyaleman wants to merge 6 commits intootland:masterfrom
reyaleman:blessings-updated
Open

blessings updated#4479
reyaleman wants to merge 6 commits intootland:masterfrom
reyaleman:blessings-updated

Conversation

@reyaleman
Copy link
Contributor

@reyaleman reyaleman commented Jun 2, 2023

Pull Request Prelude

Changes Proposed

Modified how blessings are handled in the server.

  1. moved blessings to storages and removed blessings column from players table.
  2. added missing blessings (adventurer's, twist of fate and enhanced blessings)
  3. added blessings status (little green icon next to equipment)
  4. added blessings window
  5. added blessings history
  6. blessings can have "charges" (1 charge is removed when dying)

Issues addressed:

@ghost ghost requested review from DSpeichert, MillhioreBT, nekiro and ranisalt June 2, 2023 18:58
addStorageValue(key, oldValue + count);
}

void Player::removeBlessing(uint8_t blessingId, int32_t count /* = 1*/) { addBlessing(blessingId, count * -1); }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
void Player::removeBlessing(uint8_t blessingId, int32_t count /* = 1*/) { addBlessing(blessingId, count * -1); }
void Player::removeBlessing(uint8_t blessingId, int32_t count /* = 1*/) { addBlessing(blessingId, -count); }

return 0;
}

return value;
Copy link
Member

Choose a reason for hiding this comment

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

return std::max(value, 0) and get rid of the branch

}

int LuaScriptInterface::luaPlayerHasBlessing(lua_State* L)
int LuaScriptInterface::luaPlayerAddBlessing(lua_State* L)
Copy link
Member

Choose a reason for hiding this comment

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

This can be written in pure Lua now, manipulating the storages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay... should I update the int LuaScriptInterface::luaPlayerSetStorageValue(lua_State* L) method to ignore blessings range?

if (IS_IN_KEYRANGE(key, RESERVED_RANGE)) {
	reportErrorFunc(L, fmt::format("Accessing reserved range: {:d}", key));
	pushBoolean(L, false);
	return 1;
}

Copy link
Member

Choose a reason for hiding this comment

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

No, if you do that it will be impossible to set them anyway :P

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 issue I'm having is that I cannot set blessing storages because they're reserved.

function Player.getBlessing(self, blessingId)
	return math.max(0, self:getStorageValue(PlayerStorageKeys.blessingsRangeStart + blessingId))
end

function Player.addBlessing(self, blessingId, count)
	local key, count = PlayerStorageKeys.blessingsRangeStart + blessingId, count or 1
	self:setStorageValue(key, self:getBlessing(blessingId) + count)
end

function Player.removeBlessing(self, blessingId, count)
	local count = count or 1
	self:addBlessing(blessingId, -count)
end

because of:

int LuaScriptInterface::luaPlayerSetStorageValue(lua_State* L)
{
	// player:setStorageValue(key, value)
	int32_t value = getNumber<int32_t>(L, 3);
	uint32_t key = getNumber<uint32_t>(L, 2);
	Player* player = getUserdata<Player>(L, 1);
	if (IS_IN_KEYRANGE(key, RESERVED_RANGE)) {
		reportErrorFunc(L, fmt::format("Accessing reserved range: {:d}", key));
		pushBoolean(L, false);
		return 1;
	}

	if (player) {
		player->addStorageValue(key, value);
		pushBoolean(L, true);
	} else {
		lua_pushnil(L);
	}
	return 1;
}

that's why I asked if it will be ok to skip the "reserved range" check from the Player.setStorageValue function so I can move all blessings functions to lua. 😄

Copy link
Member

@ranisalt ranisalt Jun 10, 2023

Choose a reason for hiding this comment

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

Ah sorry, I thought you were asking if the blessings storage range should be added to the protected list, not the other way around. Yes, it should be removed from the reserved range, since there is a key to access them that is very clear (PlayerStorageKeys.blessingsRangeStart).

Alternatively, use a different value for PlayerStorageKeys.blessingsRangeStart that is outside the reserved range.

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.

3 participants