martysama0134

Another annoying issue is the repeated commands. This solves that issue: def __PushLastSentenceStack(self, text): global ENABLE_LAST_SENTENCE_STACK if not ENABLE_LAST_SENTENCE_STACK: return if len(text) <= 0: return if text in chatStack: #remove duplicated elements and push the new one on top chatStack.remove(text) if len(chatStack) > LAST_SENTENCE_STACK_SIZE: chatStack.pop(0) chatStack.append(text)

I was testing it and I noticed two warnings: This return is missing. This variable is unused and so this line can be removed. Meanwhile in here I made a small refactory (unique_ptr + std::array): (I'll use TSettingsArray even in packet.h later on)

I found the bug on the v1.8.0: They save the direction in the .tga: When MarkImage.cpp loads the .tga it does ilOriginFunc(IL_ORIGIN_UPPER_LEFT); and it gets flipped. By commenting these lines directly in the IL src, it gets resolved. [Hidden Content]

In game/src/guild_manager.cpp // change: rkMarkMgr.LoadMarkIndex(); rkMarkMgr.LoadMarkImages(); rkMarkMgr.LoadSymbol(GUILD_SYMBOL_FILENAME); // to: if (!rkMarkMgr.LoadMarkIndex()) { rkMarkMgr.SaveMarkIndex(); rkMarkMgr.SaveMarkImage(0); } else rkMarkMgr.LoadMarkImages(); I think if the image .tga is missing, it will glitch it on the top when it gets resized the first time, so I'm regenerating it when it attempts to load it the first time. i.e. LoadSymbol is unused code. In game/src/MarkImage.cpp, if you want to read the correct save position: // change: printf("PutMark pos %u %ux%u\n", posMark, colMark * SGuildMark::WIDTH, rowMark * SGuildMark::HEIGHT); // to: sys_log(0, "PutMark pos %u %ux%u\n", posMark, colMark * SGuildMark::WIDTH, rowMark * SGuildMark::HEIGHT); Check if it solves the issue.

Maybe you forgot to delete mark/mark_index as well. I'm using libdevil (1.8.0) as static (server-side) for 9 years I guess. Either 32bit or 64bit, c++98 / c++20. Never got a crash about it. You can also use IL without dependencies (libpng & co) server-side, because they are not needed. [Hidden Content] Even so, I checked an 8 years old mark_0.tga. The mark_index is incremental for every new guild created. If a guild doesn't upload the flag, it remains black. It never started from the bottom (like in your case). For the lzo1x_compress crash, I'd check if they uploaded a 0kb logo, or something like that.

There has been a time where 3 fake marty were scamming people at the same time. It felt like in Eminem - The Real Slim Shady

Someone linked me this old topic, and I figured out I never noticed those sounds played twice. This is my fix: (PythonNetworkStreamPhaseGameItem.cpp) There's no need to play those sounds. Alternatively (based on penger's but via apply check): (but it's not of my taste) (PythonItem.cpp) case CItemData::USE_ABILITY_UP: if (c_rkItemData.GetValue(0) == CItemData::APPLY_MOV_SPEED || c_rkItemData.GetValue(0) == CItemData::APPLY_ATT_SPEED) return USESOUND_NONE; return USESOUND_POTION;

So now you can boost your friends even faster.

::OnClick is called when you click on the npc to open the shop, but not when you buy/sell. If you battle a mob, touch anything else, the selected npc gets swapped. Easy exploit/bug.

To do so you have to select the NPC first, but you didn't. You need to set ch->SetQuestNPCID(npc->GetVID()); first. otherwise you're calling an ambiguous npc (nullptr, another npc, or a deleted npc which makes it a dangling pointer) when you use this function: LPCHARACTER CQuestManager::GetCurrentNPCCharacterPtr() { return GetCurrentCharacterPtr() ? GetCurrentCharacterPtr()->GetQuestNPC() : nullptr; } I'll send a pull request on github later, and I'll add the npc for the mine event too. (and the emotion one if time allows me) Thanks for sharing.

Is the official counting \0 characters for the length?

Someone asked about this function, and I recreated it, but I noticed there was one already public. I made some tests, and this is the result: [Hidden Content] Bench name: BoostTest, Elapsed time: 96ms Bench name: RegexTest, Elapsed time: 1009ms @ VegaS™ Bench name: mslTest, Elapsed time: 29ms from MSL Bench name: ymirTest, Elapsed time: 56ms (created right now) Bench name: ymir2Test, Elapsed time: 69ms (created right now for old compilers) This public function doesn't support commands like /m "Wild Dog", while the classic ymir does. In the godbolt link you find the whole test code. If you want to support the " tags, you'll need something like this: void split_argument(std::string_view stArg, std::vector<std::string> & vecArgs) // backward function for lazy people { vecArgs = split_arguments(stArg); } std::vector<std::string> split_arguments(std::string_view stArg) { std::vector<std::string> vecArgs; std::string_view argument = stArg; while (true) { char first_arg[256]{}; argument = one_argument(argument.data(), first_arg, sizeof(first_arg)); vecArgs.emplace_back(first_arg); if (argument.empty() || argument[0] == '\0') break; } return vecArgs; } meanwhile for old compilers: void split_argument(const std::string & stArg, std::vector<std::string> &vecArgs) { const char * argument = stArg.data(); while (true) { char first_arg[256]{}; argument = one_argument(argument, first_arg, sizeof(first_arg)); vecArgs.push_back(first_arg); if (!argument || argument[0] == '\0') break; } } ------------------------ Dynamic arg buffer: [Hidden Content] std::vector<std::string> split_argument4(std::string_view stArg) { std::vector<std::string> vecArgs; std::string_view argument = stArg; std::string first_arg; first_arg.resize(stArg.size()+1); //+1 cuz one_argument assigns to first_arg[0] \0 if argument is empty while (true) { argument = one_argument(argument.data(), first_arg.data(), first_arg.size()); vecArgs.emplace_back(first_arg); if (argument.empty() || argument[0] == '\0') break; } return vecArgs; }

From the default code, only POINT_MAX_SP and POINT_MAX_SP are swapped. The rest is the same. Am I missing something? Small refactory: (using a table I can shorten far more) ACMD(do_stat_minus) { char arg1[256]; one_argument(argument, arg1, sizeof(arg1)); if (!*arg1) return; if (ch->IsPolymorphed()) { ch->ChatPacket(CHAT_TYPE_INFO, LC_TEXT("µÐ°© Áß¿¡´Â ´É·ÂÀ» ¿Ã¸± ¼ö ¾ø½À´Ï´Ù.")); return; } if (ch->GetPoint(POINT_STAT_RESET_COUNT) <= 0) return; auto pointId = POINT_NONE; if (!strcmp(arg1, "st")) { pointId = POINT_ST; if (ch->GetRealPoint(pointId) <= JobInitialPoints[ch->GetJob()].st) return; } else if (!strcmp(arg1, "dx")) { pointId = POINT_DX; if (ch->GetRealPoint(pointId) <= JobInitialPoints[ch->GetJob()].dx) return; } else if (!strcmp(arg1, "ht")) { pointId = POINT_HT; if (ch->GetRealPoint(pointId) <= JobInitialPoints[ch->GetJob()].ht) return; } else if (!strcmp(arg1, "iq")) { pointId = POINT_IQ; if (ch->GetRealPoint(pointId) <= JobInitialPoints[ch->GetJob()].iq) return; } else return; if (pointId == POINT_NONE) return; ch->SetRealPoint(pointId, ch->GetRealPoint(pointId) - 1); ch->SetPoint(pointId, ch->GetPoint(pointId) - 1); ch->ComputePoints(); ch->PointChange(pointId, 0); if (pointId == POINT_IQ) // HT? ch->PointChange(POINT_MAX_HP, 0); else if (pointId == POINT_HT) // IQ? ch->PointChange(POINT_MAX_SP, 0); ch->PointChange(POINT_STAT, +1); ch->PointChange(POINT_STAT_RESET_COUNT, -1); ch->ComputePoints(); }

This is its definition: template<typename T, typename... Args> std::unique_ptr<T> make_unique(Args&&... args) { return std::unique_ptr<T>(new T(std::forward<Args>(args)...)); }

I know this is an old thread. This should solve it: The return value of .erase must be stored in the it again. To be more precise: void CQuestManager::CancelServerTimers(DWORD arg) { auto it = m_mapServerTimer.begin(); while (it != m_mapServerTimer.end()) { if (it->first.second == arg) { event_cancel(&it->second); it = m_mapServerTimer.erase(it); continue; } ++it; } } if you're edgy: void CQuestManager::CancelServerTimers(DWORD arg) { for (auto it = m_mapServerTimer.begin(); it != m_mapServerTimer.end();) { if (it->first.second == arg) { event_cancel(&it->second); it = m_mapServerTimer.erase(it); } else ++it; } } or in c++20 void CancelServerTimers(uint32_t arg) { auto erase_check = [&](auto&& it) { if (it.first.second == arg) { auto event = it.second; // copy ptr event_cancel(&event); } return it.first.second == arg; }; std::erase_if(m_mapServerTimer, erase_check); } [Hidden Content]

Updated to v2.8: added --forcetype <typeId> added automatic create/load of .dat files if type 4-5 added type 6 compression updated credits

How is that offline shop related to this? Does it use the FLUSH_CACHE packet?

People may misunderstand: Someone used this exploit in a pserver, and the game admin asked many people for help (I suppose). Replacing the Flush Cache packet to Save() is enough. It's exactly the same bug ShopEx had years ago in a specific mainline branch.

If the key is missing, it will create a new entry inside the unordered_map. Any m2 map not in table will have a max distance of 0.f.

UPDATED TO V2.6 added support for unpacking type 4-5 (you need the original cshybridcrypt_<packname>.dat loaded with --load_cs <packname> before --unpack) added some samples for packing/unpacking with type 4 and type 5

Fixed.

#define ENABLE_CAMERA_ZOOM_OPTIMIZE #ifdef ENABLE_CAMERA_ZOOM_OPTIMIZE #include <unordered_map> #endif PyObject* appSetCameraMaxDistance(PyObject* poSelf, PyObject* poArgs) { float fMax{}; if (!PyTuple_GetFloat(poArgs, 0, &fMax)) return Py_BuildException(); #ifdef ENABLE_CAMERA_ZOOM_OPTIMIZE static const std::unordered_map<std::string, float> mapCameraZoom = { {"metin2_map_n_flame_dragon", 6000.0f}, {"metin2_12zi_stage", 5000.0f}, {"metin2_map_defensewave", 5000.0f}, {"metin2_map_miniboss_01", 5000.0f}, {"metin2_map_miniboss_02", 5000.0f}, {"metin2_map_mists_of_island", 5000.0f}, }; const std::string c_rstrMapFileName = CPythonBackground::Instance().GetWarpMapName(); const auto it = mapCameraZoom.find(c_rstrMapFileName); if (it != mapCameraZoom.end()) fMax = it->second; #endif CCamera::SetCameraMaxDistance(fMax); return Py_BuildNone(); } I slightly refactored it. Now it's O(1).

This vulnerability should affect every server. You can duplicate item rewards, and also crash the server through dangling pointers. The danger of this bug escalates to how many custom systems, and how many crafting quests (for example, the vitality ore quest, not the cube system) you have in your server. How to trigger it: Any quest that uses select & wait, and the item lua module after that is vulnerable. After the server uses select() or wait(), the player's quest state is suspended. After the player replies using the CG packet, the quest state is recovered. So what's wrong with it? It doesn't verify if the stored quest item ptr expired. You basically need to destroy the selected item ptr in order to dupe the rewards of the quest. After some tries, you may get a core crash in the game. (dangling pointers often cause crashes only after that memory sector has been rewritten) In my files, I've checked (since several years ago) if the quest state was suspended for the the default windows such as exchange, cube, shop. This bug can work very easily on offline shops or other new systems that don't check that. After the select() or wait() is called, you send the selected item to the (e.g.) offlineshop system window. It will delete the item ptr in the game. Now, you can press "Ok" on the quest, and the quest will proceed as if the item still existed. The item still exists in the offlineshop, but not the item ptr anymore. The item won't be deleted by the quest even after item.remove() is called. This is the fix: diff --git a/s3ll_server/Srcs/Server/game/src/char.cpp b/s3ll_server/Srcs/Server/game/src/char.cpp index 0ea307fa..65b1dd65 100644 --- a/s3ll_server/Srcs/Server/game/src/char.cpp +++ b/s3ll_server/Srcs/Server/game/src/char.cpp @@ -303,7 +303,10 @@ void CHARACTER::Initialize() m_dwQuestNPCVID = 0; m_dwQuestByVnum = 0; - m_pQuestItem = NULL; + m_dwQuestItemVID = 0; m_dwUnderGuildWarInfoMessageTime = get_dword_time()-60000; @@ -6123,33 +6126,37 @@ LPCHARACTER CHARACTER::GetQuestNPC() const void CHARACTER::SetQuestItemPtr(LPITEM item) { - m_pQuestItem = item; + m_dwQuestItemVID = (item) ? item->GetVID() : 0; } void CHARACTER::ClearQuestItemPtr() { - m_pQuestItem = NULL; + m_dwQuestItemVID = 0; } LPITEM CHARACTER::GetQuestItemPtr() const { - return m_pQuestItem; + if (!m_dwQuestItemVID) + return nullptr; + return ITEM_MANAGER::Instance().FindByVID(m_dwQuestItemVID); } diff --git a/s3ll_server/Srcs/Server/game/src/char.h b/s3ll_server/Srcs/Server/game/src/char.h index cc4da2bb..74b3470e 100644 --- a/s3ll_server/Srcs/Server/game/src/char.h +++ b/s3ll_server/Srcs/Server/game/src/char.h @@ -1674,9 +1674,9 @@ class CHARACTER : public CEntity, public CFSM, public CHorseRider private: DWORD m_dwQuestNPCVID; DWORD m_dwQuestByVnum; - LPITEM m_pQuestItem; + DWORD m_dwQuestItemVID{}; // @fixme304 (LPITEM -> DWORD) // Events To unlock the client in order to move your windows with a quest open, you edit this from interfacemodule.py: You can even Hide the TopBar and BottomBar from uiQuest.py for a better result. Important: after this fix, the item ptr may be nullptr after they press enter, so you need to check if the item ptr is still valid by using this function: ALUA(item_is_available) { auto item = CQuestManager::instance().GetCurrentItem(); lua_pushboolean(L, item != nullptr); return 1; } ... { "is_available", item_is_available }, // [return lua boolean] By simply doing: when 169.take begin local s1=select("Yes", "No") if s1==2 then return end if not item.is_available() then return end end If you want to tryhard, and be sure that the item ptr didn't get swapped, you can do as following via quest:

That function generates the _atlas.bmp by taking every generated minimap.dds 1:1. Are you trying to increase the minimap size? The ingame big map is usually created by downscaling the dumped _atlas.bmp to 256px.

I've recently moved it outside PointsInstant (just for a better design style) CHARACTER_POINT_INSTANT m_pointsInstant; std::unique_ptr<PlayerSlotT> m_PlayerSlots; and included QuickSlot as well without any issues. (just check in char_quickslot.cpp if the unique_ptr is nullptr like always) struct PlayerSlotT { std::array<LPITEM,INVENTORY_AND_EQUIP_SLOT_MAX> pItems; std::array<BYTE,INVENTORY_AND_EQUIP_SLOT_MAX> bItemGrid; std::array<LPITEM,DRAGON_SOUL_INVENTORY_MAX_NUM> pDSItems; std::array<WORD,DRAGON_SOUL_INVENTORY_MAX_NUM> wDSItemGrid; std::array<LPITEM,CUBE_MAX_NUM> pCubeItems; #ifdef ENABLE_ACCE_COSTUME_SYSTEM std::array<TItemPosEx,ACCE_WINDOW_MAX_MATERIALS> pAcceMaterials; #endif std::array<TQuickslot,QUICKSLOT_MAX_NUM> pQuickslot; }; By using std::array instead of c-arrays, you'll get these differences: You may need to use XXX.data() to get the raw ptr in few occasions. memset is now useless garbage, because XXX = {} does that job for you You get assertions if the array subindex goes out of bound You can use all the generic features std:: containers support What doesn't change is that std::array is still considered a trivial type for the template trails, and can also be used for client<>server packets.