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.

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.

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;

 

 

On 1/18/2023 at 8:21 AM, covid said:

care to explain?

So now you can boost your friends even faster.

22 minutes ago, Owsap said:

I didn't need to do that for the specific events because OnClick did that for me, so in this case there isn't any problem at all

::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.

1 hour ago, Owsap said:

, npc.get_race()

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.

31 minutes ago, Mali said:

Text length has null character too

 

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:

https://godbolt.org/z/hd6oW4j7P

Bench name: BoostTest, Elapsed time: 96ms
Bench name: RegexTest, Elapsed time: 1009ms @ VegaS™
Bench name: mslTest, Elapsed time: 29ms from

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:

https://godbolt.org/z/5cWdoxbee

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();
}

 

1 hour ago, bossy_max said:

@ martysama0134what can I use instead of make_unique ?

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:

On 12/20/2020 at 4:47 PM, Mali said:

thanks for fix

For me, I wouldn't prefer this. Because you are copying pair. Just use simple iterator like this:

for ( ; it != container.end(); ) {
	if (condition)
		it = container.erase(it);
	else
		++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);
}

https://godbolt.org/z/EEa5Yz7Mn

 

Updated to v2.8:

• added --forcetype <typeId>
• added automatic create/load of .dat files if type 4-5
• added type 6 compression
• updated credits

2 hours ago, [007]DawisHU said:

Finally fixed.

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

1 hour ago, Trial said:

saw this morning that martysama has published it on it's blog.

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.

15 minutes ago, Deso said:

fMax = mapMaxDistances[c_rstrMapFileName];

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

 

6 minutes ago, ReFresh said:

'c_rstrMapFileName': redefinition

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:

9 minutes ago, Aslan said:

Is it possible to increase the image size when saving the atlas? I also looked in part of the source and I think I found what I was looking for.

 

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.

6 hours ago, Amun said:

By using what Marty posted here

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.