Skip to content

Commit

Permalink
Deprecated SQL item, monster and monster skill databases
Browse files Browse the repository at this point in the history 
- The files are (and will) still be included for use in Control Panels
  or websites, but their use as the data source for the map server is no
  longer supported. Please upgrade to their text counterparts instead.

Signed-off-by: Haru <haru@dotalux.com>
  • Loading branch information
@MishimaHaruna
MishimaHaruna committed Aug 9, 2015
1 parent 01a5960 commit 84e02ac
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
12 changes: 6 additions & 6 deletions conf/inter-server.conf
View file
Expand Up @@ -139,13 +139,13 @@ npc_market_data_db: npc_market_data
// Hercules, while still loading the SQL tables we provide to be used
// exclusively by your Control Panel or Website.

// Use SQL for item_db? (not recommended)
use_sql_item_db: no
// Use SQL for item_db? (deprecated)
//use_sql_item_db: no

// Use SQL for mob_db? (not recommended)
use_sql_mob_db: no
// Use SQL for mob_db? (deprecated)
//use_sql_mob_db: no

// Use SQL for mob_skill_db? (not recommended)
use_sql_mob_skill_db: no
// Use SQL for mob_skill_db? (deprecated)
//use_sql_mob_skill_db: no

import: conf/import/inter_conf.txt
24 changes: 24 additions & 0 deletions src/map/map.c
View file
Expand Up @@ -3748,14 +3748,38 @@ int inter_config_read(char *cfgName) {
else if(strcmpi(w1,"use_sql_item_db")==0) {
map->db_use_sql_item_db = config_switch(w2);
ShowStatus ("Using item database as SQL: '%s'\n", w2);
if (map->db_use_sql_item_db) {
// Deprecated 2015-08-09 [Haru]
ShowWarning("Support for the SQL item database is deprecated and it will removed in future versions. "
"Please upgrade to the non-sql version as soon as possible. "
"Bug reports or pull requests concerning the SQL item database are no longer accepted.\n");
ShowInfo("Resuming in 10 seconds...\n");
sleep(10);
}
}
else if(strcmpi(w1,"use_sql_mob_db")==0) {
map->db_use_sql_mob_db = config_switch(w2);
ShowStatus ("Using monster database as SQL: '%s'\n", w2);
if (map->db_use_sql_mob_db) {
// Deprecated 2015-08-09 [Haru]
ShowWarning("Support for the SQL monster database is deprecated and it will removed in future versions. "
"Please upgrade to the non-sql version as soon as possible. "
"Bug reports or pull requests concerning the SQL monster database are no longer accepted.\n");
ShowInfo("Resuming in 10 seconds...\n");
sleep(10);
}
}
else if(strcmpi(w1,"use_sql_mob_skill_db")==0) {
map->db_use_sql_mob_skill_db = config_switch(w2);
ShowStatus ("Using monster skill database as SQL: '%s'\n", w2);
if (map->db_use_sql_mob_db) {
// Deprecated 2015-08-09 [Haru]
ShowWarning("Support for the SQL monster skill database is deprecated and it will removed in future versions. "
"Please upgrade to the non-sql version as soon as possible. "
"Bug reports or pull requests concerning the SQL monster skill database are no longer accepted.\n");
ShowInfo("Resuming in 10 seconds...\n");
sleep(10);
}
}
else if(strcmpi(w1,"autotrade_merchants_db")==0)
safestrncpy(map->autotrade_merchants_db, w2, sizeof(map->autotrade_merchants_db));
Expand Down

9 comments on commit 84e02ac

@sagunkho
Copy link
Member

Choose a reason for hiding this comment

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

Why is this being changed? why would txt file database be preferred to an sql?

@4144
Copy link
Contributor

@4144 4144 commented on 84e02ac Aug 11, 2015

Choose a reason for hiding this comment

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

I think because sql database for static data can be useless. Also it slow on load.
Another reason after monster db files will be converted to new format, some one in plugins may add new complex fields here. And this will be pain to support in db format.

With same reason can be all configs in database, but it useless too.

Probably items in database may help if use multiply servers with same db at same time, but not sure what this kind or servers exists.

For panels all this databases can be auto generated. because this here no issue.

@MishimaHaruna
Copy link
Member Author

Choose a reason for hiding this comment

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

In addition to what 4144 said, I'd like to add that, in order to add custom items to the SQL databases, even in the past, you would have needed to add them to your item_db.conf in any case, because of:

 // Note: the following databases may get out of date at times, or not be
 // thoroughly tested (if at all, since they're auto-generated). As such it is
 // not advisable to rely on them other than for informative reasons (Control
 // Panels, websites, etc.)

and we never provided updates for the sql item db changes (but only offered the entire script, so that you can drop and re-create your table).

Since maintaining two different versions of the databases (and related load functions) has become too much of a burden, and it will hinder any future item database improvements (i.e. adding new fields), we decided it's time to drop it. There was a warning about it being discouraged to use for several months anyways.

This said, if you want to use sql databases, we offer a plugin system anyone can use to extend Hercules how they see fit, without forcing us to maintain code we consider useless in the core.

@sagunkho
Copy link
Member

Choose a reason for hiding this comment

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

I understand what you're trying to do here, but an sql database (static or otherwise) will make our job a hell lot convenient, and facilitate a lot more than the current functions.

Firstly, It can be made possible to edit the data through other means (eg. website/applications), and reload data in-game for real time changes/updates. This also prevents the need for a lower priority GM to have shell access to reload or update the database. As 4144 mentioned, it can make it possible to use the same db for more than just one map server, a feature that eA was made to support from way back I believe.

Secondly, Indexing? readability and ease of use? we can easily create update statements for users to keep updated, this is a lot more convenient in cases where your custom database edits conflict with the head revision specially considering the size of the item database.

Thirdly, .conf/txt files require some extra code specially to help parse the file as opposed to sql which uses straight forward functions.

I personally feel that we should use sql as a primary method to load and store the database, let alone removing it's support. Ultimately making hercules more user-friendly. Hercules or any other emulator should give priority to facilitate these features and options in my honest opinion, because at the end of the day, guess who's using the software.

@MishimaHaruna
Copy link
Member Author

Choose a reason for hiding this comment

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

The problem here is that, by the way it works, you were never meant to edit the data directly on the SQL, since those tables were considered discardable, and every time anything was updated in the item DB, the only way to update your data was to drop and re-create the tables (and the source .sql file was generated from the textual version of the database).

It is very evident that managing those .sql tables in a source control is impossible, for any one-line edit will create a file-wide conflict. That's the reason why we moved away from the old CSV format, to a more manageable libconfig format, that produces more meaningful diffs, and is more optimized for a VCS like git.

As I said, feel free to write your own plugin to load from sql databases. We will not accept pull requests related to this.

Edit:

Also, before being a developer, I'm a Hercules user. For the past two years since I joined the developer team, I've always focused on user-friendliness and usability (and security). I personally believe we're heading in the right direction. As a user, I surely wouldn't want to go back to two years ago.

There are several ways to let other people update an item database, without having to give them shell access. You should perhaps looking into git hooks and such, to let trusted people commit to your private repository and update the copy on server. That's surely more secure than a web interface, since it easily lets you see who updated what and when, and to revert any edits with ease.

@sagunkho
Copy link
Member

Choose a reason for hiding this comment

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

To edit something in the sql db all it needs is an update / alter / replace statement, which could be committed in a new file in sql-files/upgrades just like we do for current misc sql updates (maybe in a new folder for item/mob db) is what I meant and in addition to this update the main item_db(_re).sql, so I don't see the need to erase the entire db unless doing a fresh install. And it being the main item/mob database why would it be considered discardable? What was the original intent? I'm unclear on understanding. Although I get that there were 2 dbs and we were only using the text versions for a long period, I still think going the sql way is far more beneficial on this so i figure we'll need a plugin to go with, unnecessarily. For the web based edits, I meant it would leave that option possible without having to give them access to the server files, but yes revisions would be a better way making it possible to revert if exploited.

I appreciate what you're doing and where you're taking hercules don't get me wrong, I'm just expressing my opinion and my intention is probably the same as everyone else's here.

@MishimaHaruna
Copy link
Member Author

Choose a reason for hiding this comment

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

I understand your point about providing upgrades for the SQL tables, please remember that there are two types of upgrades that would happen. The item/mob database tables are essentially data, while all other tables we provide, are empty containers for data generates by the server.

Most of (if not all) the sql upgrade scripts we have, are upgrades to the table structures, setting default values for the new and existing records (as well as altering the original creation script).

Item and mob databases would require a much more complicated approach. In the case of an update to the data, we could offer a script that updates the changed values (but that would overwrite any customizations you made, without you even noticing, whereas an upgrade to the libconfig database would generate a one-line conflict, easy to solve, for the affected line or lines).
If, in order to detect this kind of changes, you keep up to date your main item_db.sql, such a chance would easily generate a file-wide conflict (like it used to happen with the former csv version of those databases), making it almost impossible to track the conflicting line.
An example of this that you can easily reproduce is, if you edit the name field for several monsters in the mob_db (.txt or .sql, the effect is the same), and then we release an update that edits the atk for a few monsters, or adds new entries: the almost entirety of the file will be marked as conflicting, and you'll have to manually verify the changes in each and every line -- or rather, restart from scratch. If you do the same with the item db (libconfig format), chance is it will generate absolutely no conflicts, since the lines you edited and the lines we edited are separate and easily detectable by git.

An even worse case would be the update of the database structure to add a new field. That would require an upgrade script to alter the table structure, as well as a script to enter the new value for all the items that don't use the default value (think of what would have happened when, for example, we merged the item_trade_restrictions into the main item_db).
Additionally, the main sql file would need to be updated, causing the havoc described above.

Regardless of this, the rationale behind this update is that, any data generated by the server goes to sql, while any other data goes to a configuration file. As a bonus, sql backups will also be smaller, and won't need to contain megabytes of static data.

@sagunkho
Copy link
Member

Choose a reason for hiding this comment

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

Ok I understand the intention now, that clears up my doubt. Thanks man, and kudos on the latest commits.

@rarriagada
Copy link

Choose a reason for hiding this comment

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

My server is using scripts to update the item_db table in real time, the scripts injects code into some items to add extra functionality for users with some quests and properties.

We are using an extended storage service that uses the item_db with new tables to show detailed information about the items inside the game.

We have lots of querys joining tables, including item_db.

This change is not compatible with my server, congratulations.
Myzter

Please sign in to comment.