alliedmodders / sourcemod

SourceMod - Source Engine Scripting and Administration
http://www.sourcemod.net/
990 stars 425 forks source link

Introduce MemoryPointer #2196

Open Kenzzer opened 4 months ago

Kenzzer commented 4 months ago

This whole PR is still an heavy draft, nothing has been field tested.

Introduction

As of today no immediate solution to PseudoAddress has been explored. Only third-party solutions like NotnHeavy/SM-Address64 & skial-com/port64 have been made, but no major adoption so far. And while those solutions can work, I believe the friction for plugin authors is quite high. Now having to worry about the server architecture, and the confusing concept of treating ptrs as array of int/any.

Explanation

Given that there's no timeline as for when enum struct natives will ever be a thing. I propose instead to expose a new handle type that shall act as an opaque pointer towards abitrary memory regions. The underlying C++ class IMemoryPointer contained by the handle type is exposed as a public header. Allowing third-party extensions to wrap their pointers into it, no longer having to rely and depend on cell_t capacity to store the pointer's value on 32 bits.

IMemoryPointer shall become the defacto standard to pass abitrary pointers around for plugins & extensions.

List of natives

MemoryPointer.MemoryPointer(int size) Allocates an abitrary amount of memory.

MemoryPointer.Load Similar to LoadFromAddress native.

MemoryPointer.Store Similar to StoreToAddress native.

MemoryPointer.Offset Offsets the base pointer, and creates a new MemoryPointer handle.

MemoryPointer.LoadMemoryPointer Interprets the loaded data as an address and wraps it into a new MemoryPointer handle.

MemoryPointer.StoreMemoryPointer Retrieves the pointer's value wrapped inside the MemoryPointer handle, and stores it.

MemoryPointer.LoadEntityFromHandle Similar to LoadEntityFromHandleAddress native.

MemoryPointer.StoreEntityToHandle Similar to StoreEntityToHandleAddress native.

GameData.GetMemSigEx Similar to GameData.GetMemSig but wraps the pointer into a MemoryPointer handle.

GameData.GetAddressEx Similar to GameData.GetAddress but wraps the pointer into a MemoryPointer handle.

GetEntityMemoryPointer Similar to GetEntityAddress but wraps the entity pointer into a MemoryPointer handle.

SDKCall_MemoryPointer New sdkcall type to allow member function calls from the ptr contained by a MemoryPointer handle.

SDKType_MemoryPointer New sdkparam type, to retrieve the abitrary ptr contained inside a MemoryPointer handle. INVALID_HANDLE / null are allowed values to represent nullptr, if allow null flag was passed.

PrepSDKCall_SetAddressFromMemoryPointer Similar to PrepSDKCall_SetAddress native.

Ultimately this PR aims to supersede #2159 & #2112

NotnHeavy commented 4 months ago

I think this is a decent work-around, considering the current cell_t limitations of SourcePawn and the numerous extensions that currently exist as a result of 64-bit address workarounds - including mine (which I hoped would've been the standard until an official SM native of some kind came around).

However, I feel like it's still missing out on some features, such as being able to wrap around existing addresses rather than only being able to use the handle to allocate a new block of memory.

Perhaps natives for converting between pseudo addresses and MemoryPointer would also be ideal?

In the end as well this obviously creates a new issue of having to delete your MemoryPointer objects every time you use them as well, but I suppose active use of it wouldn't necessarily be encouraged.

KaelaSavia commented 4 months ago

In the end as well this obviously creates a new issue of having to delete your MemoryPointer objects every time you use them as well, but I suppose active use of it wouldn't necessarily be encouraged.

Perhaps it could be possible to have argument in MemoryPointer constructor that would decide whether pointer is freed on plugin end, or requires manual freeing.

Also, would it be possible for "reasons" to have those two natives implemented?

Other than that, current solution is pretty decent, people should be able to easily adapt. Although in future also having wiki page just like we had for transistional syntax will make things even smoother in that regard!

Kenzzer commented 4 months ago

Perhaps it could be possible to have argument in MemoryPointer constructor that would decide whether pointer is freed on plugin end, or requires manual freeing.

Purposefully leaking memory isn't something I'd advise. The standard in Sourcemod is that all plugin-instanced ressources are freed on unloading, this shouldn't be an exception. If a plugin or extension wants to keep a memorypointer around for longer, they're free to create their own and copy the data over.

NotnHeavy commented 3 months ago

There's issues with using SDKType_MemoryPointer. These are addressed in a PR I made to Kenzzer's PR.

Kenzzer commented 3 months ago

We should be good to merge this time. Though maybe we should wait for @psychonic to split branches, as mentioned on discord. Stabilise 1.12, start 1.13 and merge this over there.

NotnHeavy commented 3 months ago

The thing with 1.12 at the moment is that dvander is currently refactoring the SourcePawn compiler completely, to the point that int64 hypothetically may be introduced. I'm not sure if it will make sense to start 1.13 now.

dvander commented 3 months ago

It'll be a while. It's a slog if there ever was a slog. I'd branch if you need to.On Aug 22, 2024 10:28 AM, NotnHeavy @.***> wrote: The thing with 1.12 at the moment is that dvander is currently refactoring the SourcePawn compiler completely, to the point that int64 hypothetically may be introduced. I'm not sure if it will make sense to start 1.13 now.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

Kenzzer commented 3 months ago

The thing with 1.12 at the moment is that dvander is currently refactoring the SourcePawn compiler completely, to the point that int64 hypothetically may be introduced. I'm not sure if it will make sense to start 1.13 now.

Branching makes the most sense, because we can make breaking changes on the dev branch. This was suggested because int64 on SP is a possibility. And when it is, we can destroy MemoryPointer without really breaking bcompat, because its dev branch.

NotnHeavy commented 3 months ago

Ah, I suppose I haven't really thought about that. That makes more sense then.

NotnHeavy commented 3 weeks ago

I decided to work on these for you. My code should be fine, although I can't test on my laptop because both my laptop and internet are horrible in the place I am currently staying at.