Jump to content

Ikarus_

Developer
  • Posts

    402
  • Joined

  • Last visited

  • Days Won

    20
  • Feedback

    0%

Everything posted by Ikarus_

  1. I ve also found a really slow piece of code which execute a syscall on every call that can be avoided just by declaring the variables as statics. Here is the piece of code i m talking about: // Generate random probability for spawning something. std::random_device RandomDevice; std::mt19937 Generate(RandomDevice()); std::uniform_real_distribution<> Distribute(ShipDefense::MIN_PROB, ShipDefense::MAX_PROB); And here the fix i would recommend: // Generate random probability for spawning something. static std::random_device RandomDevice; static std::mt19937 Generate(RandomDevice()); static std::uniform_real_distribution<> Distribute(ShipDefense::MIN_PROB, ShipDefense::MAX_PROB); Here is a small benchmark i made to better clarify what we are talking about. As you can see, we are talking about an improvement that reduces execution time by about 100,000 times, not a small amount I would say. [Hidden Content] EDIT ----- I ve noticed right now that you got a failed check which should prevent numerical overflow but that actually it doesn't. //THIS EXPRESION RESULT TO BE ALWAYS FALSE DUE TO NUMERICAL OVERFLOW // Prevent second counter from overflowing. if (pSpawnEventInfo->uiCountSec > UINT_MAX) return 0; //CORRECT WAY OF PREVENTING NUMERICAL OVERFLOW // Prevent second counter from overflowing. if (static_cast<uint64_t>(pSpawnEventInfo->uiCountSec) + 1 > UINT_MAX) return 0;
  2. I leave here a few tips that could improve code readability, performance and safety. I also report you a serious issue that generates memory leaks. Clearly, even though there are things that I consider outdated (which I am sure you have seen in the ymir code written in the early 2000s), I still appreciate that you have released all this work for free. The Functor (struct with operator()) is a trick used in the past to work with temporary functions that can store data, in short, a very outdated "lambda". Since C++11, we can use lambdas to definitely replace Functors, which are clearer and more readable and probably much optimizable by the compiler. The serious issue I noticed is how you insert and remove data inside m_mapShipDefense. (ps. emplace is always preferable than insert) The raw use of new and delete is to be considered outdated, and really unsafe. Proof of this is the leak I am about to show. Looking on CShipDefenseManager::Remove method: void CShipDefenseManager::Remove(const DWORD c_dwLeaderPID) { ShipDefenseMap::iterator it = m_mapShipDefense.find(c_dwLeaderPID); if (it != m_mapShipDefense.end()) m_mapShipDefense.erase(it); //erased but not deleted? } and again looking on CShipDefenseManager::Destroy() method: void CShipDefenseManager::Destroy() { ShipDefenseMap::iterator it = m_mapShipDefense.begin(); for (; it != m_mapShipDefense.end(); ++it) { CShipDefense* pShipDefense = it->second; if (pShipDefense) { pShipDefense->Destroy(); //is this use safe if you check its value immediately below? if (pShipDefense) delete pShipDefense; } } m_mapShipDefense.clear(); } A good way to solve your issue would be the use of smart pointers which are much safer and do the dirty work for you. PS: Since C++98 you can use struct without encapsulating it in a typedef declaration. typedef struct SBroadcastAllianceHP { } TBroadcastAllianceHP; // can be changed to struct TBroadcastAllianceHP { }; I've only checked ShipDefense.cpp so far. If I get time, I'll check the rest ---- EDIT I think you have misunderstood the meaning of the return value of the c lua functions. You should return the number of pushed elements in the stack. Many of the functions I have seen in the code you shared have a wrong return. int ship_defense_mgr_land(lua_State* L) { const LPCHARACTER c_lpChar = CQuestManager::instance().GetCurrentCharacterPtr(); if (c_lpChar == nullptr) return 0; CShipDefenseManager& rkShipDefenseMgr = CShipDefenseManager::Instance(); rkShipDefenseMgr.Land(c_lpChar); return 1; // WRONG - NO PUSHED ELEMENTS SO WHY 1? } int ship_defense_mgr_is_created(lua_State* L) { const LPCHARACTER c_lpChar = CQuestManager::instance().GetCurrentCharacterPtr(); if (c_lpChar == nullptr) return 0; CShipDefenseManager& rkShipDefenseMgr = CShipDefenseManager::Instance(); lua_pushboolean(L, rkShipDefenseMgr.IsCreated(c_lpChar)); return 1; // CORRECT, YOU PUSHED 1 ELEMENT } ----- EDIT2 I would recommend to add some small check here to make the function skip all monsters dead outside the dungeon. This method CShipDefenseManager::OnKill get called 2 times (first inside CHARACTER::Dead, second inside CHARACTER_MANAGER::DestroyCharacter) anytime a monster die, anytime a monster die in the whole core.Not a good idea keep it loops throught `std::map` (which is really slow to get iterated) and perform the whole CShipDefense::DeadCharacter on each dungeon instance (which it again has a while loop iterating a `std::map`) method anytime a player kill a monster anywhere. All of this is preventable with a small check making the function skips 99.9% of the monsters killed/purged. bool CShipDefenseManager::OnKill(LPCHARACTER lpDeadChar, LPCHARACTER lpKillerChar) { if (lpDeadChar == nullptr) return false; if (m_mapShipDefense.empty()) return false; //TODO : Add here a check to skip 99.99% of the monsters which are not relevant in this method
  3. jemalloc was probably called internally by standard apis. looking on the steps listed by the backtrace: //internal call of je_large_dalloc #1 __je_large_dalloc (tsdn=0x28b11010, extent=0x0) at jemalloc_large.c:347 //call to free, standard defined built-in #2 0x0869df97 in __free (ptr=0x39b85f80) at /usr/src/contrib/jemalloc/include/jemalloc/internal/rtree.h:341 //last call inside lua library #3 0x0859bf65 in luaM_realloc (L=0x39ee2160, block=0x39b85f80, oldsize=540,
  4. Totally unlinked with the release xd If you want know more about the bug this release fix you can read the spoiler explanation. Cache doesn't match nothing here
  5. It is an .exe with no author, which isn't signed (sign an exe helps to exclude it is manipulated). There is no reason to consider it trustable, that's why some antivirus consider it not trustable, and that's why we are talking about trusting me or not. Anyway you would prefer to convert your drop config files into queries manually. I would make a python script file which convert, but it would take long long time, since i m really busy af The rule you mentioned, honestly, i dont think refer to files shared to help a user which can't compile a tool i released with source.
  6. You can do it by yourself. Executable files are dangerous only if u execute them, so you can download it and upload it on virustotal (before to run it), once it doesnt report nothing you can execute it. And i ve also specified "if u trust it" so if u dont trust me, that's your problem
  7. uncompatible Visual Studio version. I can't provide you the libs which would work with your version, sorry. If you trust it you can download the .exe directly from here.
  8. It might be part of the solution, but it's definitely not enough to completely remove the blackscreen.
  9. Download Metin2 Download Hi Guys, I m pretty sure most of you perfectly know what i m meaning for "DEVIL TOWER LAG AFTER WARP", It's a critical FPS Drop which happen very often just after a warp in a dungeon. I ve found the way to definitely fix it with a few lines short change. Just to spend a few words for those who want to understand something before copying and pasting the fix here is the explanation of the bug: Here is the code: DOWNLOAD ZIP
  10. Your send target info it's a little different and it is using std::vector<std::pair<int,int> instead of std::vector<LPITEM> In short, searching inside bool ITEM_MANAGER::CreateDropItemVector find all the call to item = ITEM_MANAGER::instance().CreateItem(vnum, count) and subsequently vec_items.push_back(item) and replace it with something like : vec_items.push_back(std::make_pair(vnum, count))
  11. Once you made gr2 to 3dsmax, it would be amazing to make the viceversa. It would be really amazing.
  12. As you can see there are some extra references, 2 more than expected. The expected result was 2 not 4. Where these references are it is not possible for me to know as I do not have all your root available to read, surely you have some AtlasWindow child that keeps its reference or something like that. The self.board was surely one of these, no longer now once you use SetCloseEvent with None. A test we may try is a recursive dump, it might highlight something but even if that leads to nothing, I think the things I can do without reading your code are done. def Destroy(self): import dbg def RecursiveDump(obj, name, tab, done_objects): tabstr = '\t' * tab dbg.TraceError(tabstr + ("START WITH DUMP : %s"%name)) try: for mem, value in vars(obj).items(): dbg.TraceError(tabstr + ("{}.{}={}".format(name,mem,value))) for mem, value in vars(obj).items(): if hasattr(value, '__dict__') and id(value) not in done_objects: done_objects.append(id(value)) RecursiveDump(value, name + '.' + mem, tab+1, done_objects) except Exception, e: dbg.TraceError("Exception while dumping %s : %s"%(name, str(e))) miniMap.UnregisterAtlasWindow() self.ClearDictionary() self.AtlasMainWindow = None self.tooltipAtlasClose = 0 self.tooltipInfo = None self.infoGuildMark = None self.board = None RecursiveDump(self, 'AtlasWindow', 0, [])
  13. Using this code you can check how many references to your AtlasWindow and AtlasWindow.board are still not moved to None. def Destroy(self): import dbg import sys self.board.SetCloseEvent(None) dbg.TraceError("AtlasWindow ref count = {}".format(sys.getrefcount(self)-1)) dbg.TraceError("AtlasWindow.board ref count = {}".format(sys.getrefcount(self.board)-1)) miniMap.UnregisterAtlasWindow() self.ClearDictionary() self.AtlasMainWindow = None self.tooltipAtlasClose = 0 self.tooltipInfo = None self.infoGuildMark = None self.board = None
  14. I posted the solution but when reloading this thread page i cannot see it at all. I m posting it once again. def Destroy(self): self.board.SetCloseEvent(None) miniMap.UnregisterAtlasWindow() self.ClearDictionary() self.AtlasMainWindow = None self.tooltipAtlasClose = 0 self.tooltipInfo = None self.infoGuildMark = None self.board = None It could solve your bug.
  15. This would solve your problem finally: def Destroy(self): self.board.SetCloseEvent(None) miniMap.UnregisterAtlasWindow() self.ClearDictionary() self.AtlasMainWindow = None self.tooltipAtlasClose = 0 self.tooltipInfo = None self.infoGuildMark = None self.board = None
  16. It looks the message edit wasn't applied successfully, my bad. So here's the code with the fix applied. def Destroy(self): import dbg references = vars(self.board) dbg.TraceError("STARTING AtlasWindow.board DUMP") for name, value in references.items(): dbg.TraceError("AtlasWindow.board.{} = {}".format(name, value)) dbg.TraceError("STARTING AtlasWindow.board.titleBar DUMP") for name, value in vars(self.board.titleBar).items(): dbg.TraceError("AtlasWindow.board.titleBar.{} = {}".format(name, value)) miniMap.UnregisterAtlasWindow() self.ClearDictionary() self.AtlasMainWindow = None self.tooltipAtlasClose = 0 self.tooltipInfo = None self.infoGuildMark = None self.board = None
  17. Look at this message, copy this code and make the test. The last test you made use an old Destroy which i corrected yesterday night (for this reason you need to copy again it)
  18. Yesterday i update the code fixing a bug, but it looks you copied it with no fix applied. Can u copy it again?
  19. What may not be cleaned is not necessarily AtlasWindow but its child named "board" . Let's see if you dump it what's coming out. def Destroy(self): import dbg references = vars(self.board) dbg.TraceError("STARTING AtlasWindow.board DUMP") for name, value in references.items(): dbg.TraceError("AtlasWindow.board.{} = {}", name, value) dbg.TraceError("STARTING AtlasWindow.board.titleBar DUMP") for name, value in vars(self.board.titleBar).items(): dbg.TraceError("AtlasWindow.board.titleBar.{} = {}", name, value) miniMap.UnregisterAtlasWindow() self.ClearDictionary() self.AtlasMainWindow = None self.tooltipAtlasClose = 0 self.tooltipInfo = None self.infoGuildMark = None self.board = None The test is obv identical to the previous one. My guess is that the close event of the "BoardWithTitleBar" isn't moved to None
  20. Can u paste your uiminimap.py in a pastebin? I would like to read it
  21. There's a easy&fast way to check if i m correct. Replace your def Destroy with mine posting here: def Destroy(self): miniMap.UnregisterAtlasWindow() self.ClearDictionary() self.AtlasMainWindow = None self.tooltipAtlasClose = 0 self.tooltipInfo = None self.infoGuildMark = None self.board = None import dbg references = vars(self) dbg.TraceError("STARTING WITH DUMPING AtlasWindow") for name, value in references.items(): dgb.TraceError("AtlasWindowReference {}={}".format(name, value)) You need to login into game and warp your character, You will find dumping inside your syserr.txt Let's hypothesize the possible scenarios: 1. The syserr looks empty : Your Destroy function is never called: You must call it 2. The syserr contains strings but all of them has value "None" or numbers/empty lists etc: My guess was wrong, the problem is something else. 3. The syserr contains strings and at least one of them is a reference to an object which makes your AtlasWindow not cleaned enough. if you need help understanding what scenario you got you can post your syserr here
  22. Memory leak. All references instantiated in the class must be set to None before deleting the object. Check your class AtlasWindow inside uiminimap.py and search for def Destroy, inside this function you have to move to None all the reference.
×
×
  • Create New...

Important Information

Terms of Use / Privacy Policy / Guidelines / We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.