Help with GetArtPath in op2ext

[Vagabond]

In the ConsoleModuleLoader, we have a function called GetArtPath that actually exists outside of the class.

Code: [Select]

int __fastcall GetArtPath(void*, int, char*, char*, char *destBuffer, int bufferSize, char *defaultValue)
{
	strcpy_s(destBuffer, bufferSize, moduleDirectory.c_str());
	return moduleDirectory.size();
}

Because GetArtPath exists outside the class, the variable moduleDirectory must also live outside the class. This means that moduleDirectory is shared between all instances of ConsoleModuleLoader and is not wiped when new versions of ConsoleModuleLoader are instantiated.

This is okay for the production use of op2ext since only one instance of ConsoleModuleLoader is needed.

Now that we have added unit tests to op2ext, the unit tests will create multiple instances of ConsoleModuleLoader to test different aspects of the code. Because moduleDirectory exists outside of the class ConsoleModuleLoader, it persists and is shared between all instances, which is not ideal.

To remedy the situation, I wanted to bring GetArtPath into the class. However, I cannot figure out how to accomplish it.

I attempted to translate GetArtPath from a calling convention of __fastcall to __thiscall. Since __fastcall passes the second argument in the EDX register, and __thiscall does not, I removed the first argument void* from the GetArtPath's signature. Don't know if this is legal, I'm out of my wheelhouse here...

I tried to remove __fastcall by rewritting the function signature as:

Code: [Select]

int ConsoleModuleLoader::GetArtPath(int, char*, char*, char *destBuffer, int bufferSize, char *defaultValue)
{
	strcpy_s(destBuffer, bufferSize, moduleDirectory.c_str());
	return moduleDirectory.size();
}

Notice that __fastcall is removed, the first argument void* is removed, and the function is now a member of ConsoleModuleLoader. I think maybe these changes keep the function signature in sync???

However, when compiling the function SetArtPath, throws 2 compile time errors

E0171   invalid type conversion   op2extStatic   \op2ext\srcStatic\ConsoleModuleLoader.cpp   114   

These errors are caused by the phrase (DWORD)(&GetArtPath). I'm not sure why I cannot cast to a DWORD once the function is a member of the class. Full function definition below for context.

Code: [Select]

void ConsoleModuleLoader::SetArtPath()
{
	// This value may also be set using the DEBUG section of the .ini file, using the property ART_PATH.
	// If set in .ini file, ART_PATH must be deleted at end of session or will persist between plays.

	// Insert hooks to make OP2 look for files in the module's directory
	// In ResManager::GetFilePath
	Op2MemSetDword((void*)0x004715C5, (DWORD)(&GetArtPath) - (GetLoadOffset() + (DWORD)0x004715C5 + sizeof(void*)));
	// In ResManager::CreateStream
	Op2MemSetDword((void*)0x00471B87, (DWORD)&GetArtPath - (GetLoadOffset() + (DWORD)0x00471B87 + sizeof(void*)));
}

I'm looking for an assist from someone with better assembly and C++ knowledge than myself.

-Brett

[Hooman]

The __fastcall attribute is a hack to simulate __thiscall from a non-member function. The other aspect at play here, is pointer to member function syntax is tricky and has a number of restrictions and undefined behavior we were never able to get around, so we had to fake it. Hence it was faked with non-member functions, where the syntax and casting is considerably easier, and the results are much more well defined.

With that said, I just found out today that MSVC supports special syntax for casting between void* and pointer to member functions. Apparently it was used in the Detours library.

As for the hack, the original method in Outpost2.exe is a class member function, so it is passed the hidden this pointer in the ECX register. A free standing method would pass all parameters on the stack. That is, unless the free standing method was declared __fastcall, in which case it would pass the first two parameters in the ECX and EDX registers. This meant the first parameter mapped onto the this pointer in ECX register when the call destination was actually a member function (even though the pointer was declared as a non-member function). The EDX part was an unpleasant detail of the hack, which resulted in an unused second parameter. The second parameter was typically just set to 0, it didn't really matter, as it was ignored by the called method, which wasn't expecting any input in EDX. As EDX is one of the trashable registers according to the calling conventions, it would simply be overwritten if the called method needed to use the register.

If you were to convert the method from __fastcall to __thiscall, you'd be getting rid of the second parameter entirely, while the first parameter would convert into the hidden this pointer which is really just the object before the period:

Code: [Select]

obj.method(param1, param2, ...);  // C++
obj_method(obj, param1, param2, ...);  // Rough C equivalent, ignoring the register passing conventions
__fastcall obj_method(obj, ignored, param1, param2);  // Hacked __fastcall method, can call C++ target

As for how to hack things with pointer to member function syntax, that's a topic for another day. Or at least a topic for code review.

[Vagabond]

Thank you for explaining again. I think it is making sense now.

I briefly looked over your newest branch in op2ext. Will try to comment on it tomorrow.

-Brett