Conversation
Any code that should be written better? I'm up to colaborate :P |
Well, in the end, by still supplying this code to this repo you come out looking like a consummate professional to me. |
Added missing worldid on config.lua and a small note |
I believe that making changes directly to the database file is not the best option, wouldn't it be more interesting to use the migration system, since most users already have an online server? Just a tip, not a criticism. |
What I don't like, is that in some places you are calling it "WorldId" and in other "ServerId" Let me show an example:
and in config there is: So, my advice is: follow some rules. Either call it world ID, or server ID. Because now it's a bit confusing. I would call it world overall. Like this:
|
Please add credit for the original author of the code @Milice https://otland.net/members/16727/ |
@Donkey-Robot |
He's kivera global, the hour of the post on his thread and this one are too similiar and it can't be just a coincidence, please try to ban him, as i'm sure that he's going just to bring shit here |
The code has been updated by the changes suggested |
schema.sql
Outdated
@@ -57,7 +56,6 @@ CREATE TABLE IF NOT EXISTS `accounts` ( | |||
|
|||
CREATE TABLE IF NOT EXISTS `players` ( | |||
`id` int(11) NOT NULL AUTO_INCREMENT, | |||
`world_id` int(11) NOT NULL DEFAULT '0', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you removed this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@omarcopires suggested a migration file for the new owners, so i made it a migration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you still need to include it in schema.sql
Schema.sql should be always up-to-date.
And migrations for old servers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and sorry, this is my first pull request xD
It's fine I suppose @aurelion5670. It's just I spent a non-trivial amount of time doing that, and now it will not be represented in the commit log in a fashion GitHub understands. Some other notes on git usage: |
this is wonderful, but we cannot merge this right now cuz we are focused to improve features and performance and probably futures features will need world id and when we reach this time we will able to merge. I will keep this open if anyone wants to use this on your own server. we appreciate your contribution! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
// Original idea and template by Milice, continued and completed by Aurelion5670 | ||
|
||
#ifndef FS_GAMEWORLDCONFIG_H_8675309REMEMBER0MILICE0COVID2019 | ||
#define FS_GAMEWORLDCONFIG_H_8675309REMEMBER0MILICE0COVID2019 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice easteregg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your welcome. 😈
@@ -0,0 +1,10 @@ | |||
function onUpdateDatabase() | |||
print("> Updating database to version 1 (world system)") | |||
db.query("ALTER TABLE `server_config` ADD `world_id` INT(11) NOT NULL DEFAULT '0' FIRST;") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this alter table fails. The column already exists on the first installation (schema.sql)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but required if you aren't starting from a fresh schema? like updating existing running server
db.query("ALTER TABLE `market_offers` ADD `world_id` INT(11) NOT NULL DEFAULT '0' AFTER `id`;") | ||
db.query("ALTER TABLE `players_online` ADD `world_id` INT(11) NOT NULL DEFAULT '0' FIRST;") | ||
db.query("ALTER TABLE `tile_store` ADD `world_id` INT(11) NOT NULL DEFAULT '0' FIRST;") | ||
return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all these alter tables will fail if doing an clean install. You must either use migration or change the schema
@@ -20,6 +20,7 @@ We use the [issue tracker on GitHub](https://github.com/opentibiabr/OTServBR-Glo | |||
- [fear lucien](https://github.com/FearLucien) | |||
- [cjaker](https://github.com/Eternal-Scripts) | |||
- [slavidodo](https://github.com/slavidodo) | |||
- [aurelion and milice for multiworld system](https://github.com/aurelion5670) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think only the maintainers are allowed to change the readme
src/iologindata.cpp
Outdated
@@ -201,7 +201,7 @@ void IOLoginData::updateOnlineStatus(uint32_t guid, bool login) | |||
|
|||
std::ostringstream query; | |||
if (login) { | |||
query << "INSERT INTO `players_online` VALUES (" << guid << ')'; | |||
query << "INSERT INTO `players_online` VALUES (" << guid << ') WHERE `world_id` = "<< g_gameserver.getWorldId() <<"'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
g_gameserver or g_gameworld?
when I login my client debug :S |
@eCov19 try to update items of the server, this PR uses an old items |
I've tried to implement on the current nostalrius distro, but the client just debugs when fetching the character list. The getCharacterList function calls for an account number, but an account name in this source. maybe theres something else required for the nostalrius account number method? Debug Assertion 7.7x Control.cpp 1174 Another I had when trying from the view file instead of compare changes Wed Jun 03 21:31:26 2020 |
@@ -0,0 +1,10 @@ | |||
function onUpdateDatabase() | |||
print("> Updating database to version 1 (world system)") | |||
db.query("ALTER TABLE `server_config` ADD `world_id` INT(11) NOT NULL DEFAULT '0' FIRST;") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but required if you aren't starting from a fresh schema? like updating existing running server
print("> Updating database to version 1 (world system)") | ||
db.query("ALTER TABLE `server_config` ADD `world_id` INT(11) NOT NULL DEFAULT '0' FIRST;") | ||
db.query("ALTER TABLE `players` ADD `world_id` INT(11) NOT NULL DEFAULT '0' AFTER `id`;") | ||
db.query("ALTER TABLE `houses` ADD `world_id` INT(11) NOT NULL DEFAULT '0' AFTER `house_id`;") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't there also be:
db.query("ALTER TABLE `houses` ADD `world_id` INT(11) NOT NULL DEFAULT '0' AFTER `house_id`;") | |
db.query("ALTER TABLE `houses` ADD `world_id` INT(11) NOT NULL DEFAULT '0' AFTER `house_id`;") |
The schema is updated to make this change, but no the live database?
|
||
PropWriteStream stream; | ||
for (const auto& it : g_game.map.houses.getHouses()) { | ||
//save house items | ||
House* house = it.second; | ||
|
||
if (house->getOwner() == 0) // CUSTOM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this save a lot of time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this could cause item cloning... If a player has a house full of items, and then takes the items out and lose ownership of the house, the house will keep its previous save state, when it still contained the player's items. And if items aren't cleaned when a new owner takes it, then they might get some cloned items?
Not the solution I'd choose for that. Splitting the database in global (with account amd some others global info) and them having db instamces per world (since each world will run a servet instance) sounds much nicer and simpler, imho. |
Do you have any documentation on implmenting something liek this? I'm just looking for options to support multiple words using the same client, so all connection info/webpage/database is the same |
@@ -37,7 +37,7 @@ function onStartup() | |||
end | |||
|
|||
local time = os.time() | |||
db.asyncQuery('TRUNCATE TABLE `players_online`') | |||
db.asyncQuery('TRUNCATE TABLE `players_online` WHERE `world_id` = '..configManager.getNumber(configKeys.WORLD_ID)..'') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TRUNCATE WHERE
? Not possible 😛 Truncate will empty whole table. Should be DELETE
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, just make it DELETE FROM instead of TRUNCATE TABLE
No, I have not. However, you simply need a few steps:
As each server instance is an 'world instance' and has it own world_db, you don't need to worry about per world customization. Site related, you can make an api that controls your website db operations. Some kind of per world router.
something like that. |
@@ -408,6 +410,7 @@ CREATE TABLE IF NOT EXISTS `guild_membership` ( | |||
|
|||
CREATE TABLE IF NOT EXISTS `houses` ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Houses Id is the primary key and house ID, with multiple worlds, wouldn't we need to import all 800 and whatever houses per world, setting the ID accordingly.
So if real map has 800 houses, 2 worlds with 2 real maps will need houses inserted twice with 2nd world ID and so on.
startupErrorMessage("Unable to load gameworlds!"); | ||
return; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
g_gameworld.setWorldId(g_config.getNumber(ConfigManager::WORLD_ID));
Otherwise getWorldId() will always be 0
not work with two servers in raid. Anyone tested it with two computers with separated database? |
This is only made for using one database just with diffrent world_id |
print("> Updating database to version 1 (world system)") | ||
db.query("ALTER TABLE `server_config` ADD `world_id` INT(11) NOT NULL DEFAULT '0' FIRST;") | ||
db.query("ALTER TABLE `players` ADD `world_id` INT(11) NOT NULL DEFAULT '0' AFTER `id`;") | ||
db.query("ALTER TABLE `houses` ADD `world_id` INT(11) NOT NULL DEFAULT '0' AFTER `house_id`;") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
db.query("ALTER TABLE `houses` ADD `world_id` INT(11) NOT NULL DEFAULT '0' AFTER `house_id`;") | |
db.query("ALTER TABLE `houses` ADD `world_id` INT(11) NOT NULL DEFAULT '0' AFTER `id`;") |
Hello. Since there are many issues to be solved, many systems to finish and new things to be done, we will close the pull, as it is very unlikely that he will go to develop. But, we are very happy with your contribution and in the future it may be reopened and reused. We hope your understanding. |
I know its closed but is this one usable? |
Yes it works fine |
Multiworld System for OTServ Global, multiworld flag is already enabled but it doesn't affect the main server at all.
What it contains?
Willing to do the neccesary changes to make it merged into the repo.