Since no one is actually talking about the code, I have some questions. Forgive me if some of them are naive; I haven't worked in C++ much.
1. Why is there no scripting engine? The point of a scripting engine is to decouple the actual scripts (language, framework, API) from the core code, and avoid a recompilation. What benefits did you see in removing it? As a follow up, you only have around 30 npc scripts; were there more when a scripting engine existed for Titan? If so how would you add the vast number of scripts that were present then or are present in Odin sources? Do you have any plans to write a language converter or transpiler?
2. I took a brief look into the code and didn't see many smart pointers - is there any reason why it's not consistent? You mentioned that you focused on performance and memory efficiency. Have you done any load testing and/or used tools like valgrind to expose memory leaks through high coverage scenarios? How can you support your claims? A consistent theme across MapleStory development is a lack of load testing tools to objectively analyze performance. Most people equate performance to memory consumption on startup, which is a highly flawed metric, and it seems like you've done the same without offering more detail.
3. I noticed the architecture/structure is actually very similar to post-RMI-removal Odin, but there are some peculiarities in where data is held. Why do you load all guilds and player names? Why are fame timestamps kept in the World server? How come online players are kept in the world, instead of in each channel? How do you efficiently send a message to only players in a certain channel?
4. I have not seen any synchronization in key parts of the code, like in packet sending and throughout Map. Am I mistaken here? If not, have you actually tested the implications of async writes per-client? I haven't worked with asio in a while, but do you have a write queue with a strand to handle this? What is the highest number of players you've had in the same map? If that number is 1-2, I expect to see a multitude of concurrency problems and unexpected behavior as that number scales.
5. I noticed in your drop expiry code you have a single task that runs every 8 (or 180) seconds that clears drops and respawns monsters. Unless I'm misunderstanding something, this does not have expected behavior, where each individual drop lasts for 180 seconds. Consider the case where the expiry task ran, and 5 seconds later, an item was dropped. In the next run, it would be there for 175 seconds and would not be removed, which means it actually stays on the map for 355 seconds instead of 180. Since the same Mob objects are used throughout the life of a Map, this will have race conditions because the code does not seem to be threadsafe, and still wouldn't simulate variable numbers of spawns from spawn points like in GMS.
6. I haven't dug into your wz loading code, but what benefits are there to load directly from the .wz files rather than another format (sql, xml)? Does the wz format have faster than O(N) lookups? What about the nx format, does that have faster than O(N) lookups? Have you explored it as a potential solution?
7. It is very unclear to me why all the packet handling code is in Player, even for login packets, even though the Player class seems to represent a logged-in Character object (with stats, guilds, etc). Is there any reason why you removed the "Client" abstraction and have this kind of implementation?
8. The PacketCreator usage seems a little disappointing; surely there is a better way to send packets than doing "PacketCreator packet26; ... PacketCreator packet37; ..." ?
Edit:
9. It also seems that there is no spawn delay for items dropped from monsters. Is this correct? The delay for dropped items need to match the duration of the monster's death animation. Is this anywhere in the source?
Thanks for your time!
Hi, thank you for the questions, which I gladly answer.
First of all I want to state that I have not taken any java/odinms source/repack as a basis for design choices and such.
Originally, I started with TitanMS2, but over the years I completly re-wrote it myself.
1. There were multiple considerations for having no scripting engine.
1.1.: It creates a dependency on another library.
1.2.: It requires creating matching script functions for the C++ functions where needed.
1.3.: Ofcourse, native is more efficient in therms of memory and speed.
It's true that there aren't very much npcs, mostly basic like job advancers and some more.
As I explained, MS features still need some work including this.
Now ofcourse this is something that can and needs to be re-valueted from time to time and different people will come to different conclusions.
Now that work focuses more on MS-Features, this will need to be re-checked again.
2. As, especially C++11 brought new features including smart pointers, auto, constexpr, and so on, I considered where and how to use those new features. There were a total of thousands of revisions, including alot of tests with smart pointers. I have deliberately created a new version control basis without old history's, just saying this addiotionally here. Anyways, I have decided to use smart pointers for some code/classes where it in my oppinion is more needed, first, for example, the Item related code. It's true that it isn't consistent yet. Though this isn't much about memory overhead in my oppinion. unique_ptr generally doesn't have overhead that really needs to be mentioned, and shared_ptr rather for reference counting. I am not using valgrind or other such tools (yet), but I do check for memory leaks and smart pointers ofcourse are, to my experience, unless there is real missusage like a dead reference to some pointer, immune to memory leaks.
I use performance testing tools, mostly "Very Sleepy", though. That tool is a profiler which shows the amount of cpu time and real time that was spent in functions and details to it, like the percentage of the total cpu time usage of function x, which helps alot.
Startup is also very fast, as I said. And that includes, as I also said, the loading of all .wz data that the server uses. Now that isn't definite proof for good performance generally, but gives some hints that that is the case, same goes for memory usage, which is very low after the startup is done.
3. I cannot comment on odinms and such structures, because I have never extensively checked that out.
Now the fame, player names and guild stuff goes back to some idea I had: Is it possible to reduce mysql querys while not loosing functionality?
With that special handling, I wanted to check that out. I think it's an interesting and working thing. It goes like this:
On server startup, guild data is loaded. Any guild data is only manipulated in the application memory. At server shutdown, the guild data is saved to mysql. Fame data is generally only held in the memory while the application runs. Now that's not fully GMS like, as then at some point the fame dates are lost, but as I said this were just some ideas that might be subject to change at some time again. And for player names: They are also loaded at startup and then worked with in memory. For some features like buddylist's and other stuff names are needed and this avoids some mysql querys. Now ofcourse this also can be questioned and I am willing to re-consider this at anytime. Actually, right now there is no way to send packets to players in a specific channel. Actually the reason is because I never ran into a case where that was needed, as far as I remember. But if you can name me ideas where that might be needed, I will consider adding this.
4. As currently it is not in fact multithreaded in therms of multiple threads being actually used (apart of 1 thread for acceptor and 1 for all handling stuff and 1 for asio timers internally by asio), no locks or whatever are needed.
My highest players online was 45 for a short time (up to 1 hour). There were no issues with lags or high cpu usage or such.
I have not used strand yet, but I am positive that the current code can handle very much players concurrently.
The asynchronous pattern has much benefits in my oppinion, even though it's full potential is not yet used due to lack of multiple worker threads currently. But it's true that intensive testing is needed for asynchronous writes and such.
5. As I said, as currently there aren't multiple worker threads, thread safety is not needed currently and not an issue.
As for the timing, you are corrent, it isn't fully GMS-like nor fully correct rigth now. I already planned to change that so drops are correctly handled and monster spawns also need to be checked to be made really GMS-like and correct. But it works well currently (especially monster spawns).
6. First I want to go into the performance related question. As I stated, there are a huge number of wz loading operations on server startup, with my current code it's very fast, therefore I accepted this as an optimal solution. I used vana's MCDB for some time and before that wz xml's, but I think direct .wz loading is the best solution for multiple reasons. Though, I currently cannot state what Big O is as I have yet to calculate and properly find it out.
Pro's of .wz loading include:
* no need for additional conversation/steps/files in between, usage of the official files with it's content
* with my code, loading it is very fast
* this allows an, in my oppinion, rather easy version change: sql (for example mcdb) is stuck to specific coloumns and such, therefore, this needs changes often, and just like wz xmls it needs to be extracted first, which again needs a tool on it's own
These are the primary reasons for it.
7. The Player class indeed combines very much code and it could have been done different, but I think the way it is done is acceptable, even though I take skeptical views on this seriously.
8. Actually I re-coded packetcreator not long ago while in private, I think this way is the best, even though I agree the variable naming looks a bit weird.
You create a PacketCreator object, which itself allocates some memory. Then you write packet data on that memory, if needed, it allocates more with realloc. Then it's send to one or more clients, including copying the packet bytes to a new buffer which is allocated for an async write operation to the client. By RAII, once the scope is left, PacketCreator object is destroyed and it's destructor deallocates the packet memory. I think this is quite a good solution, but I welcome suggestions from anyone, including you.
9. I will need to further check how it is in GMS first, before commenting this point.
I hope I could answer the questions well enough, thanks for your interest and you can ofcourse answer to this and react to my answers point by point.
Greetings
Buya