otland / forgottenserver

A free and open-source MMORPG server emulator written in C++
https://otland.net
GNU General Public License v2.0
1.59k stars 1.06k forks source link

src folder organization #2186

Open jo3bingham opened 7 years ago

jo3bingham commented 7 years ago

I feel as though it's high-time the source files in the src folder were organized into meaningful subfolders. For example, connection, lockfree, networkmessage, outputmessage, protocol, protocolgame, protocollogin, protocolold, protocolstatus, rsa, and waitlist could all be grouped together in a network folder.

This isn't high-priority, or even necessary for that matter, but it may be worth it in the long run. Especially as more source files are added to the project.

DSpeichert commented 7 years ago

I would primarily see "luascript.cpp" as "lua/*.cpp", which is already suggested in #2169 so it's not a new idea. I guess the rest would follow as needed.

ranisalt commented 7 years ago

It is actually pretty hard because files are very much interconnected with includes and there will be a lot of .. and ../.. out there, but would be very good. I think it would be even better to split the server into smaller objects instead of a single target, them link together. CMake excels at that.

DSpeichert commented 7 years ago

Not sure what you mean by splitting into smaller objects in this case, but if it's something that could help drive the required RAM to compile down, then it could solve the other issue.

diath commented 7 years ago

@ranisalt You can use the include_directories directive to avoid relative paths. For instance you if you had map/tile.cpp and wanted to include a network related header you could just use #include "network/whatever.h" rather than #include "../network/whatever.h" and so on.

ranisalt commented 7 years ago

@DSpeichert e.g. building all protocol related stuff (protocol*.(cpp|h) files) in a small library libprotocol.a, then linking against tfs executable. It should help using less RAM, too.

@diath that would become a mess of relative paths. I do prefer paths relative to the current file than to some arbitraty root folder. The more include directories, the more filename colision you might have.

djarek commented 7 years ago

Because this is such a large task I've decided that the best course of action will be to split it up into smaller parts. The drawback is that there will be an interim period during which there might be a little bit of a mess, but I think it's a much better approach than one humongous patch. I'd like to ask you guys to try and split this task up into multiple issues so that each issue can be resolved with a fairly small patch (e.g. 1st issue and patch will group the lua module into separate files, the 2nd will group up networking). I've created a project so that we can group all these related issues into it and track progress.

foxrother commented 5 years ago

People still working on/interested in this?

jo3bingham commented 5 years ago

@foxrother it should be done, and there's a project for it, so I would say there's definitely interest. However, I haven't seen any work related to it, so feel free to go for it if that's what you're intending.

foxrother commented 5 years ago

@jo3bingham thought someone could be working on it silently. Not the case, apparently. :P I'm still figuring out the source code (the what, how and why), so if you guys have any additional insights on the back of your heads, please share them 'cause I'm very interested. 😁

jo3bingham commented 5 years ago

@foxrother @ranisalt said he was working on #2169, and was assigned to it, but that was 20 months ago. In an ideal world, people would assign themselves to an issue before working on it so as not to clash with someone else.

Also, I agree with this comment, this issue (or epic, in this case) should be split up into individual issues like #2169. If you want to do the Lua, you should pick that issue up. If you want to do another area, you should create a new issue, link it to the project, and assign yourself. Apart from creating new issues, you probably can't do the other things so it would be best to ask for those things in the issue comment.

foxrother commented 5 years ago

@jo3bingham there are not many active contributors as of late (sadly & understandably), so even though I saw and agree with these, I had to ask it here, (since this issue shares its name with the project and seems to be where this enhancement got more attention).

I'll assign these to myself soon, thank you for explaining that.

The broader discussion on the whole src folder belongs to here, though, right? As you pointed out, you guys already thought of creating separate network and lua folders. If you have any other thoughts on what else could be bundled in separate folders, it would be nice to say here. Right now I'm really not in a position to suggest these things, but I'd like to be seen as a resource to delegate tasks to (since you guys are busy). Some rough directions would suffice.

jo3bingham commented 5 years ago

The broader discussion on the whole src folder belongs to here, though, right?

Yeah.

If you have any other thoughts on what else could be bundled in separate folders, it would be nice to say here.

IO stuff seems like a good candidate. Maybe Creature and related classes (Player/Monster/NPC)? But Lua is the biggest one; luascript.cpp is completely out-of-hand and is only getting worse.

MillhioreBT commented 3 years ago

@DSpeichert e.g. building all protocol related stuff (protocol*.(cpp|h) files) in a small library libprotocol.a, then linking against tfs executable. It should help using less RAM, too.

@diath that would become a mess of relative paths. I do prefer paths relative to the current file than to some arbitraty root folder. The more include directories, the more filename colision you might have.

Any clearer example? Although the issue seems old, it does not take away from the fact that we should pay attention to it and fix it as soon as possible. I like the idea of ​​dividing everything into subfolders, but as you mentioned, it would be a mess of files that doesn't look very pretty.

@ranisalt If you still exist, I hope so, uwu, give us an example of your idea, someone could work on it, for example with luascript.cpp

Currently it can be divided into two smaller files, one to establish all the functions of the interface and another to establish the functions registered in lua, but that would only remove a few lines from the current file, however this add a new file to the src folder, which does not seem like a very nice solution

Kamenuvol commented 3 years ago

luascript.cpp should be implemented like otclient

MillhioreBT commented 3 years ago

luascript.cpp should be implemented like otclient

It seems to be neater, although there are many functions that cannot be declared in such a way, if I am wrong, correct me

ranisalt commented 3 years ago

@ranisalt If you still exist, I hope so

Barely

Currently it can be divided into two smaller files, one to establish all the functions of the interface and another to establish the functions registered in lua, but that would only remove a few lines from the current file, however this add a new file to the src folder, which does not seem like a very nice solution

This is already a good start. It will probably be impossible to do much more without a deep rewrite due to the unorganized way that file has grown. I would probably use something like sol2 in the future to hide most of the guts in a dependency.

MillhioreBT commented 3 years ago

@ranisalt If you still exist, I hope so

Barely

Currently it can be divided into two smaller files, one to establish all the functions of the interface and another to establish the functions registered in lua, but that would only remove a few lines from the current file, however this add a new file to the src folder, which does not seem like a very nice solution

This is already a good start. It will probably be impossible to do much more without a deep rewrite due to the unorganized way that file has grown. I would probably use something like sol2 in the future to hide most of the guts in a dependency.

I've read a lot about sol2 it looks beautiful and compact, this would reduce thousands of lines, correct? In this case, I would like to help you write all the functions related to the lua interface. the current problem I have is that sol2 seems to have errors when compiling, have you already tried this? I mean compiling the project and including sol2 in the libraries?

First of all it forces you to compile with /std:c++17 then in the compilation it throws errors that many functions that use this library are obsolete/deprecated in /std:c++17, it literally contradicts itself, these libraries are outdated and it does not seem to be alive

It also throws other errors, related to a totally outdated library, if you have a working example that works well, please show it to me, I would like to see the sun in action, and not only in theory

ranisalt commented 3 years ago

I've read a lot about sol2 it looks beautiful and compact, this would reduce thousands of lines, correct?

Definitely.

have you already tried this? I mean compiling the project and including sol2 in the libraries?

I have compiled the examples in the sol2 repository with no error. As you said, it requires C++17, but that's for good, since it will allow us to use a lot of new nice features (I'm eyeing string_view mostly). Other than that, no issues at all, it compiles cleanly.

Note that I'm on gcc 10.2 with sol2 3.2.2

MillhioreBT commented 3 years ago

I've read a lot about sol2 it looks beautiful and compact, this would reduce thousands of lines, correct?

Definitely.

have you already tried this? I mean compiling the project and including sol2 in the libraries?

I have compiled the examples in the sol2 repository with no error. As you said, it requires C++17, but that's for good, since it will allow us to use a lot of new nice features (I'm eyeing string_view mostly). Other than that, no issues at all, it compiles cleanly.

Note that I'm on gcc 10.2 with sol2 3.2.2

I understand, it is very likely that for linux system there are no problems, however for windows there are, or at least for me if there are, I use the following command to install the sol2 libraries on my system: (o_o I guess you are on linux ...)

./vcpkg install --triplet x64-windows sol2 Everything installed correctly, but I am suffering from the errors I mentioned above

btw this string_view feature is very interesting and can be handled a lot with it. It would be nice to completely replace the use of const std::string& with this! per example! we can even call like this: using R_String = std::string_view;

I hope someone can provide a way to compile with sol2 on windows and provide the correct commands for the installation, I would like to create a branch to start changing the entire lua interface from scratch

ranisalt commented 3 years ago

Is there another channel where I can relay you some ideas? I want to keep the chat, but I feel like this is dragging off-topic in this issue, I would rather talk to you directly somewhere else and come back when ideas are settled. I'm ranisalt#3449 on Discord (though I don't open it very often) or @ranisalt on Twitter and Telegram.