Problem applying end scene hook

[wag321]

I've written an endscene hook, got the basic idea from whitemagic (thanks apoc)
I'm having some problems with it though, first I have an issue calling the original endscene when my endscene tries to it crashes the game, and secondly I can only get my endscene ran if I step through my code in the debugger, if I just run it it doesn't even run the first time but crashes the game in another manner. Anyway code below, I guess I'm doing something stupid but I've been staring at it for a while now

public class Hook
    {
        [UnmanagedFunctionPointer(CallingConvention.Winapi)]
        private delegate IntPtr EndSceneDelegate(IntPtr instance);
        private static readonly EndSceneDelegate DEL_Original = EndScene;
        private static readonly EndSceneDelegate DEL_New = (EndSceneDelegate)Marshal.GetDelegateForFunctionPointer((IntPtr)Interaction.DMA.GeneralFunctions.GetEndscene(), typeof(EndSceneDelegate));
        private static readonly uint PTR_Original = (uint)Marshal.GetFunctionPointerForDelegate(DEL_New);
        private static readonly uint PTR_New = (uint)Marshal.GetFunctionPointerForDelegate(DEL_Original);

        private static readonly byte[] OriginalBytes = Interaction.DMA.RAW.ReadBytes((uint)PTR_Original, 6);
        private static readonly byte[] BTS_NewAddress = BitConverter.GetBytes(PTR_New);
        private static object MEMLOCK = new object();
        private static List<byte> ReplacedBytes = new List<byte>();

        private static IntPtr EndScene(IntPtr instance)
        {
            lock (MEMLOCK)
            {
                Remove();
                DEL_New.DynamicInvoke(instance);
                Apply();
                return instance;
            }
        }        
        public static void create()
        {
            ReplacedBytes.Add(0x68);
            ReplacedBytes.AddRange(BTS_NewAddress);
            ReplacedBytes.Add(0xC3);
            Apply();
        }
        private static void Apply()
        {
            lock (MEMLOCK)
            {
                Interaction.DMA.RAW.WriteBytes(PTR_Original, ReplacedBytes.ToArray());
            }
        }
        private static void Remove()
        {
            lock (MEMLOCK)
            {
                Interaction.DMA.RAW.WriteBytes(PTR_Original, OriginalBytes);
            }
        }
    }

[adaephon]

Currently you're getting a managed delegate for the EndScene function pointer

DEL_New = (EndSceneDelegate)Marshal.GetDelegateForFunctionPointer((IntPtr)Interaction.DMA.GeneralFunctions.GetEndscene(), typeof(EndSceneDelegate));

and then getting your unmanaged pointer to that function by recalling a Marshal method.

PTR_Original = (uint)Marshal.GetFunctionPointerForDelegate(DEL_New);

Firstly, you already have your unmanaged pointer for the original function (it's the pointer you passed to Marshal.GetDelegateForFunctionPointer in the line above. Secondly, doing it doubly like this I'm fairly sure is not supported and will give inaccurate results (read the remarks here: Marshal.GetDelegateForFunctionPointer Method (System.Runtime.InteropServices)).

On another note, your naming is kind of all over the place, making the code a bit more obscure. Some advice would be to stick with a common naming convention (why are some variables all caps (MEMLOCK), some just caps_ prefixed (PTR_New) and some just standard (ReplacedBytes)?) Also, your DEL_Original points to your custom EndScene method (the new one), not the original (that's DEL_New), but then your PTR_New points to your new method, not the original, and PTR_Original points to the original. At the very least they should be consistent, and ideally they should be named to accurately reflect what they actually are.

[streppel]

I might be wrong about this, but as you are passing the first parameter called instance as intptr I guess that the function is a ThisCall function. You make it be a winapi.And seriously take a look at your naming

[adaephon]

Normally it would be ThisCall but Windows specifically declares these methods as winapi which is just a typedef for stdcall I think

[wag321]

Thanks guys, I've cleaned up the naming and the pointers and delegates being opposites was accidental so that's fixed.
I've played around with it a bit and I'm now having a problem calling the original - it doesn't return and wow crashes.
My endscene looks like

        private static IntPtr EndScene(IntPtr instance)
        {
            Remove();
            System.Windows.Forms.MessageBox.Show("Hiya");
            IntPtr res = (IntPtr)delOriginal.DynamicInvoke(instance);
            //Apply();
            return res;
        }

So I thought that there's a problem with the way I'm removing the hook, so then when I create it I added a remove straight after apply which works fine my Endscene never get's called as you would expect. Or else my delOriginal is bad

delOriginal = (EndSceneDelegate)Marshal.GetDelegateForFunctionPointer((IntPtr)ptrOriginal, typeof(EndSceneDelegate));

But I know that ptrOriginal is good, Am I missing something here?

[_Mike]

I might be wrong about this, but as you are passing the first parameter called instance as intptr I guess that the function is a ThisCall function. You make it be a winapi.And seriously take a look at your naming

DirectX object methods are stdcall because COM interfaces are supposed to be language neutral. If they had used thiscall instead it would have limited the number of languages that could use them.
Edit:
Have a look at the interface declarations in the d3d9.h header file if you want more details. If you use D3D from C it emulates OOP by creating a "vtable" that is a struct of function pointers.

[wag321]

Edit. Nvm got it