[2016-04-16] introduction of private headers

[hemagx 70]

Hello ~

 

Rationale:

 

For long time we had many functions or variables that were supposed to be kept privately used, but because of the design of our header files were kept a public access. This caused, on one hand, poor design choices (too much binding between separate components), and on the other hand, excessively long long re-compile for the whole project if anything changed in this headers (because cause any or all files can be affected). Now with private headers only the file meant to use this header will be recompiled resulting in less compile times, waaay less!

 

Contents:

 

The login server client interface (lclif) was chosen as a working prototype of this, and now has a private interface (lclif.p.h) alongside its public interface (lclif.h).

 

Details:

 

Currently we will start limit the private function/variable/databases to a private interface under a private header only meant to be used by the module itself. This will stop other modules from accessing the private function/variable/databases (lclif.p.h should only be included by lclif.c, but never included by other .c files), however plugins will still be able to access it if they need to (the private interface is fully HPM and HPMHooking compliant).

 

The private interface is exposed through an opaque pointer inside the public interface.

 

the private data can be accessed through the interface through the private interface pointer

interface->p->private_function(); //General example
lclif->p->parse_sub(fd, sd); // Example for login client interface

 

to be able to use the private interface into your plugin please make sure to include the private header !

It's recommended to still include the public interface as well.

 

//General example
#include "project/file.h"
#include "project/file.p.h"
 
// Example to include Login Client Interface Private header
#include "login/lclif.h"
#include "login/lclif.p.h"

Merge Date:

Sat, 16 Apr 2016 15:37:47 +0200

 

Related Pull Requests:

- #1255 - https://github.com/HerculesWS/Hercules/pull/1255 - Login clif rewrite [hemagx]

 

Related Commits:

- 79d9ace - https://github.com/HerculesWS/Hercules/commit/79d9ace - Wed, 30 Mar 2016 23:58:17 +0200 Added support for private headers to the HPMDataCheck/HPMHooking generators [Haru]

- c0178d2 - https://github.com/HerculesWS/Hercules/commit/c0178d2 - Thu, 31 Mar 2016 00:16:02 +0200 HPM Hooks Update [Haru]

- ab42483 - https://github.com/HerculesWS/Hercules/commit/ab42483 - Thu, 31 Mar 2016 14:42:56 +0200 Updated GNU Make build system to support private headers [Haru]

- 7555700 - https://github.com/HerculesWS/Hercules/commit/7555700 - Mon, 28 Mar 2016 21:54:46 +0200 Rewrite client interface for login server (part 7) [hemagx]

- 37cc46c - https://github.com/HerculesWS/Hercules/commit/37cc46c - Thu, 31 Mar 2016 00:16:48 +0200 HPM Hooks Update [Haru]

- 15c9710 - https://github.com/HerculesWS/Hercules/commit/15c9710 - Sat, 16 Apr 2016 15:21:18 +0200 Moved packet_db to the private interface of lclif [Haru]

- 8448e3f - https://github.com/HerculesWS/Hercules/commit/8448e3f - Sat, 16 Apr 2016 15:22:21 +0200 HPM Hooks Update [Haru]

- bbcb040 - https://github.com/HerculesWS/Hercules/commit/bbcb040 - Sat, 16 Apr 2016 15:37:47 +0200 Merge pull request #1255 from HerculesWS/login-clif_rewrite [ibrahem Hossam]

[Skyline 23]

Cool update. 

[Protos 203]

Great work! looking forward to more architectural enhancements.

[Conflicts 10]

I  am one of those who rely heavily on login modification, needless to say this broke compatibility, for what seemingly is a structural improvement (read: pointless).

Hercules started out to be the more adventurous one, unafraid to customize and add new functionalities to the emulator (HPM / Packet Obfus among others), embracing custom unofficial stuff, and due to that, Hercules is almost always ahead of the project it has been forked off of. But look what it has become now.. It's not a school project, you wont get better grades for making the emulator as cleanly written as possible. This is a made-up statistic but I assume < 5% of the members care about standardization and (or) optimization. Main thing is, make sure it works.

 

Don't get me wrong, I can see the improvement in recompile time. But let's be honest, how often does an average RO server admin recompile? Believe me, the improvement is irrelevant. That's just my 2 cents.

And this meme sums up how I feel about what Hercules has become;

Edited April 20, 2016 by Conflicts

[Haru 291]

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.

[bWolfie 140]

I don't understand what's going on, but Hercules emulator is my favorite so more power to ya!