Jump to content

Haru

Administrators
  • Content Count

    381
  • Joined

  • Last visited

  • Days Won

    38

Everything posted by Haru

  1. 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
  2. Kenpachi's fix for this is included in the v2020.03.08+2 release I just pushed, thank you!
  3. I just pushed an update to the plugin to make it compatible with the latest Hercules version. Please test it.
  4. 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!
  5. 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.
  6. 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
  7. 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.
  8. Fixed in https://github.com/HerculesWS/Hercules/commit/2c60e01501cdce2fb18eba0fa07f6a350ae96d5e
  9. 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.
  10. 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)
  11. 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
  12. 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 :
  13. - 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?
  14. 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.
  15. 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.
  16. The reason why no one works on this "feature" is that, while being an official feature, it has nothing to do with the game. Its sole purpose is to monetize (which certainly isn't what players are looking for). If a private server owner wants to monetize (from a game they don't even own the rights for), they can invest some time, and develop a VIP system (and why not, make a pull request, we'd likely merge it, as long as it wouldn't impair the Hercules performance for those who want a vanilla Ragnarok experience). Hercules development is progressing (see the number of branches that are being worked on -- it's not only the feature that make it into master), but no one, including Ind, would want to spend their free time on something whose sole purpose is to make leechers earn money more easily. And I believe Ind was especially bitter about that aspect of the community, when he left.
  17. For the time being, you can disable strict mode in your MySQL server configuration, and it'll still allow that default value. To disable strict mode, please check your MySQL server's documentation (you'll have to edit your my.ini configuration file).
  18. No commits to the master branch doesn't mean there wasn't any development activity! There will be some commits soon, don't worry
  19. Rationale: This changeset offers improvements to the HPMHooking, making it capable to detect, at compile time, an error in the type of a hook function, as well as allowing pre-hooks to be more powerful when it comes to pointer-type arguments. Contents: The HPMHooking macros addHookPre() and addHookPost() have been slightly edited, and they can now detect if the type of the passed function is the correct type for the hooked function. In order to do so, the HPMHookingGen script produces one more header (HPMHooking.Defs.inc) that lists the hook function types. This means that, if a plugin hooks into a function through HPMHooking, and the core function changes, the plugin will show a compile-time warning instead of silently compiling (and crashing at runtime or causing undesired effects). The post-hook function types have been simplified, dropping all the extra indirection levels that were added originally. The pre-hook function types have been changed, increasing the indirection level for pointers (now all variable types require an extra '*' in pre-hooks). This makes it possible to override const pointers from pre-hooks. Impact: Scripts that use the HPMHooking will need some small syntax changes. Details: All plugins that want to use the HPMHooking will need to #include "plugins/HPMHooking.h" (it's recommended to include it just above HPMDataCheck.h) #include "plugins/HPMHooking.h" // Included by plugins that use the HPMHooking #include "common/HPMDataCheck.h" // Included by all plugins Then the addHookPre() and addHookPost() calls need to be updated to the new syntax, separating interface name and function name:/* Before */ HPExport void plugin_init (void) { addHookPre("pc->dropitem", my_pc_dropitem_pre); addHookPost("pc->dropitem", my_pc_dropitem_post); } /* Now */ HPExport void plugin_init (void) { addHookPre(pc, dropitem, my_pc_dropitem_pre); addHookPost(pc, dropitem, my_pc_dropitem_post); } Pre-hook functions will need an additional indirection level in their pointer-type arguments:/* Hooked function: */ int (*dropitem) (struct map_session_data *sd, int n, int amount); /* Pre-hook (before) */ int my_pc_dropitem_pre(struct map_session_data *sd, int *n, int *amount) // Only adds '*' to the non-pointers /* Pre-hook (after) */ int my_pc_dropitem_pre(struct map_session_data **sd, int *n, int *amount) // Adds '*' to everything Note: arguments of type va_list do not require an additional indirection level. 'va_list ap' remains 'va_list ap' and does not become 'va_list *ap' Post-hook functions will no longer need any additional indirection level in their arguments: /* Hooked function: */ int (*dropitem) (struct map_session_data *sd, int n, int amount); /* Post-hook (before) */ int my_pc_dropitem_post(int retVal, struct map_session_data *sd, int *n, int *amount) // Adds '*' to the non-pointers /* Post-hook (after) */ int my_pc_dropitem_post(int retVal, struct map_session_data *sd, int n, int amount) // No longer adds any '*' Merge Date:Sun, 1 May 2016 20:22:03 +0300 Related Pull Requests: - #1253 - https://github.com/HerculesWS/Hercules/pull/1253 - HPMHooking improvements [Haru] Related Commits: 1ec9328 - https://github.com/HerculesWS/Hercules/commit/1ec9328 - Sun, 28 Feb 2016 02:12:48 +0100 Moved HPMHooking-related definitions to plugins/HPMHooking.h [Haru] 5db7c79 - https://github.com/HerculesWS/Hercules/commit/5db7c79 - Sun, 28 Feb 2016 02:17:21 +0100 Added type-checking for the addHookPre() and addHookPost() macros [Haru] 4e49441 - https://github.com/HerculesWS/Hercules/commit/4e49441 - Sun, 28 Feb 2016 02:20:40 +0100 HPM Hooks Update [Haru] 2788afc - https://github.com/HerculesWS/Hercules/commit/2788afc - Sun, 28 Feb 2016 02:40:15 +0100 Replaced memset with braced initializers in the HPMHooking hook handlers [Haru] fa2f2f4 - https://github.com/HerculesWS/Hercules/commit/fa2f2f4 - Sun, 28 Feb 2016 02:41:01 +0100 HPM Hooks Update [Haru] 8aacecc - https://github.com/HerculesWS/Hercules/commit/8aacecc - Fri, 15 Apr 2016 19:37:54 +0200 Removed extra indirection level in HPMHooking post-hooks [Haru] 7eb4ae4 - https://github.com/HerculesWS/Hercules/commit/7eb4ae4 - Sun, 17 Apr 2016 00:38:37 +0200 HPM Hooks Update [Haru] 89e0550 - https://github.com/HerculesWS/Hercules/commit/89e0550 - Sun, 28 Feb 2016 02:48:47 +0100 Added one level of indirection to all variables in pre-hook functions [Haru] e9c98a1 - https://github.com/HerculesWS/Hercules/commit/e9c98a1 - Sun, 28 Feb 2016 02:50:40 +0100 HPM Hooks Update [Haru] 95b4e32 - https://github.com/HerculesWS/Hercules/commit/95b4e32 - Sun, 1 May 2016 20:22:03 +0300 Merge pull request #1253 from HerculesWS/hpmhooking [Andrei Karas]
  20. That page, just like most of the stuff in the wiki is severely outdated and contains misleading information unfortunately (it should be rewritten from scratch). For the time being, I patched it up a little so that it's less misleading.
  21. Rationale: This is according to our Supported Platforms policy. For an overview of supported OSes and compilers, please see the wiki page https://github.com/HerculesWS/Hercules/wiki/Supported-Platforms Contents: VS2015 is our primary target compiler on Windows, and this merge removes all the warnings that were present when compiling with that version of Visual Studio. VS2010 isn't one of our supported platforms (and hasn't been for a while), so maintaining its solution inside the repository adds unnecessary work to the dev team. Impact: While it's currently still possible to build on VS2010 (download the files `Hercules-10.sln` and `vcproj-10/*` from an older snapshot of the repository such as https://github.com/HerculesWS/Hercules/tree/bbcb040 and put them in your Hercules directory), we offer no support for any build issues causd by future commits, nor we'll update the project/solution files. Merge Date: Sun, 24 Apr 2016 13:13:19 +0200 Related Pull Requests: - #1264 - https://github.com/HerculesWS/Hercules/pull/1264 - V2015 improved compatibility and dropped VS2010 solution [Haru] Related Commits: - a92fa36 - https://github.com/HerculesWS/Hercules/commit/a92fa36 - Sun, 17 Apr 2016 13:37:08 +0200 Updated README file [Haru] - e69e8c5 - https://github.com/HerculesWS/Hercules/commit/e69e8c5 - Sun, 17 Apr 2016 13:43:30 +0200 Removed VS2010 project [Haru] - 3af03d2 - https://github.com/HerculesWS/Hercules/commit/3af03d2 - Tue, 19 Apr 2016 11:32:04 +0200 Improved parsing of the server name/port in the irc bot configuration [Haru] - 2b1dce1 - https://github.com/HerculesWS/Hercules/commit/2b1dce1 - Tue, 19 Apr 2016 15:26:36 +0200 Changed map_session_data::chatID to int (and renamed to chat_id) [Haru] - dc23fd3 - https://github.com/HerculesWS/Hercules/commit/dc23fd3 - Tue, 19 Apr 2016 16:31:57 +0200 Corrected the type for several variables through the code [Haru] - e4feddf - https://github.com/HerculesWS/Hercules/commit/e4feddf - Wed, 20 Apr 2016 15:40:19 +0200 Corrected the type of the 'length' argument of various broadcast-related functions [Haru] - d7ffa6a - https://github.com/HerculesWS/Hercules/commit/d7ffa6a - Wed, 20 Apr 2016 17:06:18 +0200 Removed the 'len' argument from clif_disp_onlyself() and clif->disp_message() [Haru] - 4788c81 - https://github.com/HerculesWS/Hercules/commit/4788c81 - Thu, 21 Apr 2016 20:38:18 +0200 Removed the 'len' argument from various message-related functions [Haru] - f5b88f9 - https://github.com/HerculesWS/Hercules/commit/f5b88f9 - Wed, 20 Apr 2016 17:23:03 +0200 Corrected the type of the 'length' argument of other message-related functions [Haru] - 9e58db4 - https://github.com/HerculesWS/Hercules/commit/9e58db4 - Wed, 20 Apr 2016 17:24:37 +0200 Added some files specific to VS2015 (Update 2) to gitignore [Haru] - 9fdb456 - https://github.com/HerculesWS/Hercules/commit/9fdb456 - Thu, 21 Apr 2016 20:57:56 +0200 HPM Hooks Update [Haru] - 31e27a1 - https://github.com/HerculesWS/Hercules/commit/31e27a1 - Sun, 24 Apr 2016 13:13:19 +0200 Merge pull request #1264 from HerculesWS/vs2015 [ibrahem Hossam]
  22. Opinions like these are what drove the project to the situation it is in right now. Through the years we have accumulated a level of technical debt that I'm not confident we'll ever be able to fill. If we keep implementing new features the way we did until now, the project will turn into a big ball of mud (or rather, it already is). Architectural changes are very needed. Now, the change the broke compatibility with your patches, isn't this one (the change in order to implement the private headers is almost irrelevant in terms of changed lines of code). What you should blame is, instead, the redesign of the client interface in the login server. In my optinion, both changes were necessary in order for the project to go forward. I'll point out some of the reasons why I want changes like these: - The private interfaces: the change may seem pointless if the only reason is compile time (in fact, they aren't pointless - even if server owners don't recompile it often, we developers do -- and by shortening the compilation time, you increase the amount of tests we're likely to do on a patch before submitting it, or reduce the time we take to fully implement and test something). There are, though, other more important reasons. By making some methods private, we're ensuring that they aren't called by other modules, reducing the chance that new features will be implemented in a half-assed way like it happened in the past (resulting in a cleaner and more manageable codebase overall). Once again, keep in mind that the changes to the lclif.c file that were necessary in order to implement this were almost zero (just some lclif->xxxx or lclif_xxxx replaced by lclif->p->xxx), so this is very unlikely to break any of your patches. - The clif changes: This is important for at least three different reasons. Firstly, right now we're mixing login logic with packet sending logic, and that's just plain wrong (login code should be independent from the client, and clif code should be client-specific, just like it's important to separate logic from presentation in any well designed application). Secondly, implementing new packets, or editing old ones has become much easier and less prone to very subtle errors (think of 'WFIFOW(fd, 13 + 7 * i + 4) = foo;' versus 'packet->item[7].nameid = foo;'). Finally, it addressed some long standing issues that made it very hard for HPM plugins to hook into packet handling. Writing a plugin that performs additional actions on a specific packet (or that completely overrides the handling of a packet) is much easier now. I already merged these changes into my own (private) codebase, where I have very large changes to the login interface, and the merge wasn't as painful as you describe it (you should consider doing it commit by commit rather than all at once - that's what I did when I merged it, and that's the reason why I refused to merge this as one mega-commit). On a side note, have you considered implementing your changes as a HPM plugin? If not, we'd like to hear the reasons, so that we can improve the HPM so that it's flexible enough for most use cases.
  23. There is usually little to no reason to install an outdated version (unless your hardware isn't supported by the latest). This means that you should go for Debian 8 or CentOS 7. CentOS 6.x is a bad choice, because it comes with a version of gcc that we don't offer support for (it may work or may break at times), and you should upgrade it on your own.
  24. Both distributions are perfectly usable. The main differences is that CentOS carries very outdated software (or stable, as they call it). I personally find debian easier to use and manage, and more compatible with third party software (especially with modern software).
  25. I'd like to add a note here: Object oriented has nothing to do with C or C++. You can write object-oriented code in C, just like you can write non-object-oriented code in C++. There are several reasons not to use C++, including the terrible choices that were made by the language designers, when they tried to engineer solutions for non-problems, or the fact that the majority of C++ developers only know about 20% of the language (too bad, the 20% they know, is a different 20% for each of them). If Hercules was written in C++, I wouldn't even have considered joining the project (it's more than enough having to cope with that absurd language during my day job). For some interesting (even if a bit outdated perhaps) reads: Linus Torvalds - http://thread.gmane.org/gmane.comp.version-control.git/57643/focus=57918 Yossi Kreinin - http://yosefk.com/c++fqa/ Now, back to the main question - why Hercules isn't written using an object-oriented paradigm. That's a question that the original developers might be able to answer, but I'm not. I'm only able to guess, they wrote it the way they knew to. Some parts of Hercules are object-oriented though. See the DBMap (common/db.c, common/db.h) for an example.
×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.