Haru

It's safe to ignore that assertion for now, or use https://github.com/HerculesWS/Hercules/pull/2877 (just merged to master) which will fix it

Kenpachi's fix for this is included in the v2020.03.08+2 release I just pushed, thank you!

I just pushed an update to the plugin to make it compatible with the latest Hercules version. Please test it.

About Code Review and Why You'd Want Your Code to Be Reviewed Hello, fellow developers and code contributors! As you certainly know, years ago, Hercules adopted a workflow based on pull requests, that includes code review as one of the necessary steps before any new piece of code makes it into the master branch of the repository. While being an uncommon and somewhat controversial change in Hercules (and in the RO emulator scene in general), code review is part of the workflow of most software projects, both open source and closed source, and has many benefits. Why Code Review The benefits of code review are several: "Given enough eyeballs, all bugs are shallow" [Linus's Law by Eric S. Raymond -- The Cathedral and the Bazaar, 1999]. While the law is not strictly true, it's certainly true that the more developers read and analyze a piece of code, the more likely it is that bugs that might be hidden in it are discovered early. Testing is not enough. It's very hard (or in the case of our codebase just plain impossible) to cover all the possible edge cases when testing a new feature or a fix. An additional pair of eyes reading the code may help discovering those more easily. This includes cases where the client would normally prevent a certain thing from happening, but it's not ensured anywhere on the server side. Better quality of code. By having other developers read a piece of code, they'll end up wondering why a certain approach was taken, rather than another, and discuss it with the submitter, leading to better, more efficient algorithms, or better engineered code. Better documentation. Since the code needs to be read by other people, it'll require proper comments (or they'll ask for explanations about the parts they can't easily explain). This increases the chance that the author, or anyone else that will need to read the same code again months or years after it's been submitted, will be able to understand it again, by finding appropriate comments in the appropriate parts of the code. Better insight into the code across the team. By reading code from different parts of the emulator as part of the review process, every team member increases their own general knowledge of the software, bit by bit. This is a very efficient way of learning how different parts of the emulator work, and why they were implemented that way. Future-proofing. By having public reviews, we keep a permanent trace of what were the hot topics and why certain decisions were taken, when a certain part of the emulator was implemented. If a bug arises, or something needs to be redesigned in future, we can look up the associated pull request and related discussion, and learn more about the discussion that went on in the past, and what's hiding behind code design decisions. Reviewing code from other people, as well as having one's own code reviewed by others might not be easy for everyone, especially at the beginning, but please try your best. Here are some suggestions on how to approach code review from either side. How to approach code review (for code authors) As a code author, the worst thing you can do is to be afraid or shy about other people judging your code. This is the wrong approach! Don't be shy, have your code looked at by others, have them praise you for your genial approach to tackle a problem, listen to their suggestions on how to improve it. But be ready to defend your implementation, if you believe it's better than the suggestions you receive, or if the critics that are moved against it are wrong or meaningless. Always keep in mind that: Having your code reviewed and commented on isn't humiliating. Other people are spending their time looking at your code, asking you why you did something in a certain way rather than another, suggesting improvements. Both sides have a lot to learn from each others. (On the other hand, if no one reviews your code, that's somewhat humiliating!) If someone spots an issue in your code, it doesn't mean that you're a bad developer. We all make mistakes, and we should be happy to learn from them (and it's definitely better if someone spots them and points them out to us before it's too late and they were able to do some harm). Never, ever, take code review personally. No one will laugh about you, fire you, kill you, shame you, etc. if your code is commented on. If you believe you're right and the comments you received are pointless or wrong, chance is that you really are right. Be ready to defend your reasons, it's possible that the reviewer didn't think of them. It is your duty to explain them your reasons. How to approach code review (for code reviewers) Reviewing code is several orders of magnitude harder than having your own code reviewed. You have to check the code for several classes of problems, point out your findings, suggest improvements. And you still have to deal with the worry about hurting the code author's feelings when pointing out a mistake. Here are some things you should keep in mind when reading and reviewing code from other people: You're not judging a person. You're judging code. Don't make your review sound personal. Always think of uncommon and edge cases, and never assume they can't happen, unless there's an explicit check that makes them impossible to happen. Even if the code was tested by the author, it doesn't mean that it can't cause problems to other existing features, or have some issues the author couldn't think of. If the same person writes and tests a piece of code, the chance that they don't test the cases they forgot to handle while coding, is very close to 100%. If the code is not following the project's style guidelines (and this isn't just about indentation, but also about names, conventions about function calls, proper modularization, etc), it is your duty to point it out now, before it's merged. This will make the life of your fellow developers easier later on. Think defensively. Consider the code you have in front of you as buggy until you can prove its correctness. If you see that a sanity check is missing, ask the author to add it. If you believe that a function returns the wrong value in certain cases, even if very unlikely to occur, prepare an example of input for which that happens and point it out. Remember that threats such as overflows, underflows, buffer overruns, null pointers, invalid pointers, numeric (floating point) approximation, etc. are always behind the corner, check for them as often as possible and prove that they can't occur. And remember that, while the code author isn't your enemy (and code review shouldn't generate negative feelings), it's often a good idea to think of them as your "professional enemies". There's a chance that something nasty is hiding in their code, even if they didn't write it with ill intent, and as such, you shouldn't blindly trust the code, regardless of who the author is. Don't be afraid when you comment on other people's code. Your goal isn't to hurt their feelings, you're asking them for explanations and/or suggesting the way you would have done something. Likewise, don't be afraid of making a pointless comment. If the author has a good reason for their implementation, be ready to take back your comment and learn from them. Don't accept compromises. If you're firmly convinced that the author's defense of their code is wrong, your duty is to prove them wrong. But if they manage to convince you, don't be ashamed of admitting you were wrong. Happy reviewing!

This is something I started working on a while ago (but I had to suspend it in order to take care of some other urgent issues). The reason why I wanted to split it is another: the current .po file is very large, and it crashes (or is rejected by) various .po editors (both online and offline). My idea was to generate, instead, one .po for each .txt file in the scripts folder (and load them when loading each .txt). This would also have a positive side-effect of only loading the strings that are actually needed (and potentially warn you if a .po is missing). Preliminary work to support this was started in the split-huld branch in my fork of the repository (commit 908d531f. So far only the generation is handled, the loading part is not yet implemented. If there's interest for that, I'll resume working on it.

But by doing that we'd lose the only reason why we decided to use the .pot/.po file format: compatibility with a large number of translation software we can currently use (i.e. poedit) and websites (i.e. crowdin, transifex. We have to stick to the syntax allowed by the gettext .po file format: https://www.gnu.org/software/gettext/manual/html_node/PO-Files.html

This wouldn't work with the current translation system though (it's a known limitation I don't have a workaround available for) Given a piece of code like this: if (Sex == SEX_MALE) mes("Welcome to Prontera"); else mes("Welcome to Prontera"); the HULD generator would detect that those strings are the same, and would just produce one entry in the .pot file, making it impossible to translate them differently. Normally, .pot files use the context field to differentiate between multiple entries that might have the same value, but we (ab)use that field to contain the NPC name instead. It's a choice that was done in order to be able to automatically generate reasonably meaningful translation templates without having to edit every script to manually add context to them.

Fixed in https://github.com/HerculesWS/Hercules/commit/2c60e01501cdce2fb18eba0fa07f6a350ae96d5e

I agree with you that libconfig is quite bad, but that's what we're stuck with for the time being. If it was up to me, I'd convert everything to yaml, but I was too late to the party, and Hercules had already started using libconfig for some files. The old .txt format had a lot of (parsing and future extensibility) problems, and it was overdue for an update. I disagree with your point about the sql_connection. The most common case is to have everything in one database, and now you can do that by just editing one file. The format allows for more advanced cases using different databases of course, and it's not much harder to achieve than it used to be (or possibly easier, since the format is standardized, and it makes easier to find things, just do a folder-wide search for sql_connection and you'll find all the files that support separate configurations. You'd previously have to search for sql, db_ and possibly other words. About the separate folders, it might be personal preference, but it feels better organized this way. If you're using any decent/modern text editor, you'd normally open the whole config folder with it, and the files to edit are just one click or keypress away, tidily organized in a nice folder tree (Vim/GVim/MacVim (screenshot), Atom (screenshot), Kate, TextMate, Sublime Text, Coda and possibly many more allow you to do that. If you're stuck with the old notepad++ and it still forces you to open all files separately, maybe it's time to start looking around for alternatives) On a side note, the files in the import folder are still all in one place. Could the file format and structure be done better? Probably. Are we still in time to do improve it? Of course. Got specific suggestions? Post them in the bugtracker. Complaining that something sucks is pointless, just say how you'd improve it.

Doxygen is a developer-only dependency. We use it (check the 'hooks' target in the top directory Makefile) only to re-generate the HPM Hooking definitions and the HPMDataCheck definitions. That operation is only needed when you edit something in the interfaces (and it's generally not needed to do manually if you push code to the repository or make a pull request, because the build-bot will take care of re-generating those after each push to the master branch) It's not needed at all in order to run Hercules or to load HPM plugins (even if they use the HPM Hooking) About vim plugins and configurations, this is a good start: https://github.com/HerculesWS/StaffPlugins/tree/master/Haru/vimsyntaxgen it's a HPM plugin that will let you generate syntax definitions, filetype detectors, etc for Hercules scripts. It also includes a syntax definition for Syntastic, in case you want to use the Hercules built-in script checker to test the syntax of your scripts every time you save the file. Note that this last part needs the syntastic vim plugin available at https://github.com/scrooloose/syntastic and some configuration (you'll need to tell it where your compiled Hercules is located: " from my .vimrc let g:syntastic_herc_compiler = '/usr/local/Hercules/script-checker' Other noteworthy plugins that I use: - airline - https://github.com/bling/vim-airline (a better status bar) - commentary - https://github.com/tpope/vim-commentary.git (comment in/out code quickly with 'gc') - DirDiff - https://github.com/grota/DirDiff.vim.git (diff directories) - easy-align - https://github.com/junegunn/vim-easy-align.git (auto-align definition blocks, such as enum definitions) - fugitive - https://github.com/tpope/vim-fugitive.git (git commands from inside vim) - gitgutter - https://github.com/airblade/vim-gitgutter.git (show markers for added/removed/edited lines if a file is in a git repository) - repeat - https://github.com/tpope/vim-repeat.git (extends the '.' repeat feature with support for more commands) - signature - https://github.com/kshenoy/vim-signature.git (show visual hints for 'm' markers) - unite - https://github.com/Shougo/unite.vim (a framework to search/show/select various kinds of things - advanced stuff, check their page for examples of use) - YouCompleteMe - https://valloric.github.io/YouCompleteMe (very good autocompleter that actually works, even with Hercules)

We don't really have any archive of actual Ragnarok database data (it's quite unfortunate, yep). Since many Hercules developers have/had access/owned a private server at some point in our life, we have some private samples we can use for tests. I generally use a clean database though. As for packet replays, it depends on what you're working on. I either gather it myself from official servers, if it's something I can access, or ask others if they can record something... There are also some packet structure/length tables available, and most stuff we implement comes from those (followed by test s with the actual client, where possible) - you can find some packet information here http://herc.ws/wiki/Packets (and if you google for some of those IDs, you can likely find more) You can find various more or less useful tools around the Downloads area (http://herc.ws/board/files/) and the Support&Releases area (http://herc.ws/board/forum/13-support-releases/), but there isn't very much tailored to source development. My main friends are vim and the dozen of general purpose vim plugins I use, along with a gcc-compatible compiler and the almighty Z-shell. For in-game testing, I tend to either download a pre-hexed client if I'm testing a specific version that I haven't made on my own, or download a clean one and hex it with NEMO. That goes on a case by case basis Probably not the kind of answer you expected, but I guess this is it at least for me

In order to specify the " character, please prefix it with a backspace (\"). This is from the libconfig documentation at http://www.hyperrealm.com/libconfig/libconfig_manual.html#String-Values :

- Did you edit them in conf/char/char-server.conf? - What do you have in your conf/import/char-server.conf? - Any [Error] or [Warning] messages in the char server console?

You have to override the global settings from inter-server.conf. You need to copy the content of global/sql_connections.conf and paste it inside the "log: { }" block that you see in inter-server.conf (and delete the @include directive that you find there). Please see doc/global_configuration.txt for more details and an example.

Rationale: This changeset updates the syntax of the configuration files, to allow for more flexibility, and get rid of our some times buggy custom parser. Contents: All the txt-based configuration files in the conf folder have been replaced with more modular files that use the libconfig syntax. This allows more expressivity in the configuration syntax (i.e it allows to use arrays/lists/groups, especially in settings such as the starting items for players, or the hash check), but, more importantly, it allows to modularize and cross-reference some settings (for example, to change a database username and password, an user only needs to edit one file now, which is in turn included by the other files). Impact: Users will need to update their custom configuration. A tool that parses the old configuration and suggests appropriate edits, is included. Details: Some highlights about the improvements of the new syntax: The start items/start point syntax (char-server.conf) is now improved as follows: // Old syntax // Start point, Map name followed by coordinates (x,y) start_point_re: iz_int,97,90 start_point_pre: new_1-1,53,111 // Starting items for new characters // Format is: id1,quantity1,stackable1,idN,quantityN,stackableN // stackable: // 0 - Not stackable (weapon, armor, egg, pet armor) // 1 - Stackable start_items: 1201,1,0,2301,1,0 // Starting zeny for new characters start_zeny: 0 // New syntax player: { new: { // Start point (Renewal) start_point_re: { map: "iz_int" x: 97 y: 90 } // Start point (Pre-Renewal) start_point_pre: { map: "new_1-1" x: 53 y: 111 } // Starting items for new characters //{ // id: Item id // amount: Item amount // loc: Item position, same as in item_db if you want the item to be equipped, otherwise 0 (optional) // stackable: Is stackable? (not stackable item types: weapon, armor, egg, pet armor) //}, start_items: ( { id: 1201 // Knife amount: 1 loc: 2 stackable: false }, { id: 2301 // Cotton_Shirt amount: 1 loc: 16 stackable: false }, ) // Starting zeny zeny: 0 } The new format is certainly more verbose, but it's much easier to understand (and allows for new fields, see for example the 'loc' optional field of the starting items. Another important feature is the 'global configuration' mechanism. There are files in conf/global/, that are included by all three (login, char, map) default configurations. For example, to globally edit the sql connection parameters for all three servers, an user can simply edit the configuration in conf/global/sql_connection.conf: sql_connection: { // [INTER] You can specify the codepage to use in your mySQL tables here. // (Note that this feature requires MySQL 4.1+) //default_codepage: "" // [LOGIN] Is `userid` in account_db case sensitive? //case_sensitive: false // For IPs, ideally under linux, you want to use localhost instead of 127.0.0.1. // Under windows, you want to use 127.0.0.1. If you see a message like // "Can't connect to local MySQL server through socket '/tmp/mysql.sock' (2)" // and you have localhost, switch it to 127.0.0.1 db_hostname: "127.0.0.1" db_port: 3306 db_username: "ragnarok" db_password: "ragnarok" db_database: "ragnarok" //codepage:"" } In order to migrate the old configuration to the new format, a helper tool is provided in tools/configconverter.pl. The script will parse any existing config files with the old format, and suggest the corresponding new format. The user will need to manually paste the suggestions in the appropriate configuration file (we decided not to automate this, so that the settings are validated by a human, rather than blindly copied over). The helper tool is smart enough to understand which settings were left to their default setting in the old configuration file, and ignore them, only printing the relevant information. % perl tools/configconverter.pl =============== Hercules Configuration Migration Helper =============== = _ _ _ = = | | | | | | = = | |_| | ___ _ __ ___ _ _| | ___ ___ = = | _ |/ _ \ '__/ __| | | | |/ _ \/ __| = = | | | | __/ | | (__| |_| | | __/\__ \ = = \_| |_/\___|_| \___|\__,_|_|\___||___/ = ======================================================================= This tool will assist you through the migration of the old (txt-based) configuration files to the new (libconfig-based) format. Please follow the displayed instructions. ======================================================================= Checking conf/char-server.conf... Ok Checking conf/import/char_conf.txt... Old file is still present 1 non-default settings found. Please review and migrate the settings as described, then delete the file 'conf/import/char_conf.txt', as it is no longer used by Hercules. - Found setting: 'pincode_enabled'. Please manually move the setting to 'char-server.conf' as in the following example: char_configuration: { pincode: { enabled: false } } Checking conf/inter-server.conf... Ok Checking conf/import/inter_conf.txt... Old file is still present 0 non-default settings found. The file 'conf/import/inter_conf.txt' is no longer used by Hercules and can be deleted. Checking conf/login-server.conf... Ok Checking conf/import/login_conf.txt... Old file is still present 0 non-default settings found. The file 'conf/import/login_conf.txt' is no longer used by Hercules and can be deleted. Checking conf/map-server.conf... Ok Checking conf/import/map_conf.txt... Old file is still present 0 non-default settings found. The file 'conf/import/map_conf.txt' is no longer used by Hercules and can be deleted. Checking conf/logs.conf... Ok Checking conf/import/log_conf.txt... Old file is still present 0 non-default settings found. The file 'conf/import/log_conf.txt' is no longer used by Hercules and can be deleted. Checking conf/script.conf... Ok Checking conf/import/script_conf.txt... Old file is still present 0 non-default settings found. The file 'conf/import/script_conf.txt' is no longer used by Hercules and can be deleted. Checking conf/packet.conf... Ok Checking conf/import/packet_conf.txt... Old file is still present 0 non-default settings found. The file 'conf/import/packet_conf.txt' is no longer used by Hercules and can be deleted. Checking conf/battle.conf... Ok Checking conf/battle/battle.conf... Ok Checking conf/battle/client.conf... Ok Checking conf/battle/drops.conf... Ok Checking conf/battle/exp.conf... Ok Checking conf/battle/gm.conf... Ok Checking conf/battle/guild.conf... Ok Checking conf/battle/battleground.conf... Ok Checking conf/battle/items.conf... Ok Checking conf/battle/monster.conf... Ok Checking conf/battle/party.conf... Ok Checking conf/battle/pet.conf... Ok Checking conf/battle/homunc.conf... Ok Checking conf/battle/player.conf... Ok Checking conf/battle/skill.conf... Ok Checking conf/battle/status.conf... Ok Checking conf/battle/feature.conf... Ok Checking conf/battle/misc.conf... Ok Checking conf/import/battle_conf.txt... Old file is still present 0 non-default settings found. The file 'conf/import/battle_conf.txt' is no longer used by Hercules and can be deleted. Merge Date:Sat, 20 Aug 2016 16:27:11 +0300 Related Pull Requests: - #1399 - https://github.com/HerculesWS/Hercules/pull/1399 - Ported the configuration to libconfig format [Haru] Related Commits: 6be7aab - https://github.com/HerculesWS/Hercules/commit/6be7aab - Mon, 15 Feb 2016 16:19:38 +0100 Renamed some char and inter server variables [Haru] c84a447 - https://github.com/HerculesWS/Hercules/commit/c84a447 - Sun, 6 Sep 2015 18:08:14 +0200 Ported char-server.conf to libconfig [Haru] 9783ce1 - https://github.com/HerculesWS/Hercules/commit/9783ce1 - Sun, 6 Sep 2015 17:58:28 +0200 HPM Hooks Update [Haru] ea9ceb1 - https://github.com/HerculesWS/Hercules/commit/ea9ceb1 - Mon, 7 Sep 2015 01:42:31 +0200 Ported inter-server.conf to libconfig [Haru] 3c84a4d - https://github.com/HerculesWS/Hercules/commit/3c84a4d - Mon, 7 Sep 2015 01:39:50 +0200 HPM Hooks Update [Haru] f56264d - https://github.com/HerculesWS/Hercules/commit/f56264d - Thu, 11 Feb 2016 00:41:54 +0100 Ported login-server.conf to libconfig [Haru] 9d70a6f - https://github.com/HerculesWS/Hercules/commit/9d70a6f - Sun, 21 Feb 2016 01:02:00 +0100 HPM Hooks Update [Haru] 85d1088 - https://github.com/HerculesWS/Hercules/commit/85d1088 - Thu, 11 Feb 2016 13:28:18 +0100 Ported map-server.conf to libconfig [Haru] e370cc4 - https://github.com/HerculesWS/Hercules/commit/e370cc4 - Thu, 11 Feb 2016 13:39:49 +0100 HPM Hooks Update [Haru] 86dde24 - https://github.com/HerculesWS/Hercules/commit/86dde24 - Thu, 11 Feb 2016 15:15:33 +0100 Improved map list loading [Haru] 654bfa0 - https://github.com/HerculesWS/Hercules/commit/654bfa0 - Thu, 11 Feb 2016 15:29:27 +0100 HPM Hooks Update [Haru] dc2130b - https://github.com/HerculesWS/Hercules/commit/dc2130b - Sat, 19 Jul 2014 18:17:59 +0200 Updated Travis script to work with the new configuration files [Haru] 5b983fc - https://github.com/HerculesWS/Hercules/commit/5b983fc - Thu, 11 Feb 2016 16:35:08 +0100 Ported npc config to libconfig [Haru] 998b48e - https://github.com/HerculesWS/Hercules/commit/998b48e - Thu, 11 Feb 2016 16:38:26 +0100 HPM Hooks Update [Haru] 1746627 - https://github.com/HerculesWS/Hercules/commit/1746627 - Thu, 11 Feb 2016 18:53:58 +0100 Ported logs.conf to libconfig [Haru] 3f85b17 - https://github.com/HerculesWS/Hercules/commit/3f85b17 - Thu, 11 Feb 2016 18:57:33 +0100 HPM Hooks Update [Haru] aa3a3f4 - https://github.com/HerculesWS/Hercules/commit/aa3a3f4 - Fri, 12 Feb 2016 12:43:14 +0100 Ported script.conf to libconfig [Haru] 8ecf562 - https://github.com/HerculesWS/Hercules/commit/8ecf562 - Fri, 12 Feb 2016 12:49:19 +0100 HPM Hooks Update [Haru] 445a68f - https://github.com/HerculesWS/Hercules/commit/445a68f - Sun, 14 Feb 2016 01:00:00 +0100 Removed unnecessary typedefs in socket.c [Haru] a8ca27d - https://github.com/HerculesWS/Hercules/commit/a8ca27d - Sun, 14 Feb 2016 01:09:21 +0100 Changed access_allow and access_deny to VECTORs [Haru] c0e59c8 - https://github.com/HerculesWS/Hercules/commit/c0e59c8 - Fri, 12 Feb 2016 16:17:30 +0100 Ported socket.conf (was packet.conf) to libconfig [Haru] 6cdb1e5 - https://github.com/HerculesWS/Hercules/commit/6cdb1e5 - Sat, 13 Feb 2016 15:38:23 +0100 Ported battle.conf to libconfig [Haru] 67a84ce - https://github.com/HerculesWS/Hercules/commit/67a84ce - Sun, 17 Apr 2016 03:10:29 +0200 HPM Hooks Update [Haru] 4e5b040 - https://github.com/HerculesWS/Hercules/commit/4e5b040 - Tue, 2 Aug 2016 01:35:41 +0200 Added option to make plugin-defined battle config entries optional [Haru] 9e02b4e - https://github.com/HerculesWS/Hercules/commit/9e02b4e - Sat, 20 Aug 2016 15:18:14 +0200 Updated references to the old config in the documentation [Haru] af77eec - https://github.com/HerculesWS/Hercules/commit/af77eec - Sat, 20 Aug 2016 16:27:11 +0300 Merge pull request #1399 from HerculesWS/settings_libconfig [Andrei Karas] Trivia This change was started by our fellow developer Panikon back in 2014, and never completed until recently (September 2015, February 2016, April 2016, August 2016), when I restarted working on it.