sa-mp-fixes icon indicating copy to clipboard operation
sa-mp-fixes copied to clipboard

Plugin

Open Y-Less opened this issue 7 years ago • 26 comments

Since the discussion for my pull request #42 got seriously side-tracked, I figured I'd make a new issue for this. Many of the fixes would be much easier as a plugin - managing state across multiple scripts that may or may not be loaded/unloaded at any given moment is just horrible! The suggestion there was a separate plugin; however, there is some overlap with YSF.

YSF is more focussed on new features and "completing" the native functions set, rather than fixing bugs (making two letters of its name now quite inaccurate, though I do very much appreciate what @Kurta999 is doing), but things like GetPlayerColor are fixed by it (I don't know what else, that was the main bug fix I found that was a new feature). Given the actual very small amount of overlap, despite the apparent similarities, I'd say to not merge, but am not too bothered either way.

In theory, since YSF should be the last plugin loaded, and this should probably be the first so that all other calls are redirected to it, there's no reason why they couldn't just work together. There's also a lot of useful code in YSF that could be reused, such as native redirection (using subhook, which I've also used before for a project nothing to do with SA:MP). I've asked in the topic about its license for using that code. As far as I am concerned, it is now fully Kurta999's project and nothing what so ever to do with me - I officially pass on any residual copyright claims I may have had due to starting it to him, should that be needed.

Y-Less avatar Jul 16 '17 15:07 Y-Less

The code is MPL, so we can use it just fine (as long as any modifications to individual files are fed back to the parent project).

Y-Less avatar Jul 16 '17 15:07 Y-Less

I don't really interested in SAMP so much anymore, but I like this idea. If somebody would develop this plugin, we could agree that move fixes from YSF to that plugin. Or if somebody would like to "continue" YSF, I can give her access to the repo.

From my opinion, best idea would be merge SKY + YSF + this plugin and rename it as SA-MP Server Addon or something like this. Now it's a bit problematic if there are 3 or more memory hacking plugin. Let's assume that two plugin wants to hook the same function -> server crashed.

ghost avatar Jul 16 '17 15:07 ghost

Well I think this is the time to merge all this while @kurta999 wants to. Also this will allow us to use a extremely more complicated name if we merge the 2 lol.

rt-2 avatar Jul 16 '17 15:07 rt-2

Y_Less: (I don't know what else, that was the main bug fix I found that was a new feature). Given the actual very small amount of overlap, despite the apparent similarities, I'd say to not merge, but am not too bothered either way.

  • https://github.com/kurta999/YSF/blob/YSF_/src/natives/fixes.cpp

Not much

ghost avatar Jul 16 '17 15:07 ghost

Ok, yeah, that's very little duplicated effort then.

Y-Less avatar Jul 16 '17 15:07 Y-Less

So, it turns out I already did this:

https://github.com/search?utf8=%E2%9C%93&q=fixes2&type=

That's not even YSF, that's a different fixes plugin I wrote and totally forgot about till just now (I was testing my own website and saw it listed as a release, totally not thinking about this at the time).

Y-Less avatar Jul 17 '17 07:07 Y-Less

Aw, yeah, I can remember about it. I preferred Dan's TimerFix Plugin instead of the Fixes2 Plugin because all the Fixes2 plugin offered was timers fixing and that callback for server messages. I didn't need to track the server messages, so I used TimerFix Plugin because it offered lots of great functions for timers. The Fixes2 plugin could be updated to offer the fixes of this include, as it was pretty much forgotten I guess. Also, the OnServerMessage callback should be removed, as it is a feature that uses memory hacking - or just redirects print functions to the plugin (?) - and it is already provided in YSF.

IstuntmanI avatar Jul 17 '17 14:07 IstuntmanI

I can't even remember what purpose it served, I think there were some messages about problems that are only printed in the console and not given in callbacks - that was probably why. If it is in YSF, then yes there's no need to do it. On the other hand, there are much better ways of doing things like that now without direct raw memory accesses and pointers - they were the only problem with memory hacking, not the fact that it was being done.

Y-Less avatar Jul 17 '17 15:07 Y-Less

OK, I've almost got a simple stable plugin base up. You can already write code like this:

HOOK(SetPlayerPos, bool(int playerid, float x, float y, float z))
{
	int
		ret = SetPlayerPos(playerid, x, y, z),
		index = GetPlayerAnimationIndex(playerid);
	switch (index)
	{
		case 958:
		case 959:
		case 960:
		case 961:
		case 962:
		case 1134:
		{
			int
				slot,
				weapons[_FIXES_WEAPON_SLOTS],
				ammo[_FIXES_WEAPON_SLOTS];
			for (slot = 0; slot < elemsof (weapons); slot++)
			{
				GetPlayerWeaponData(playerid, slot, &weapons[slot], &ammo[slot]);
			}
			weapons[11] = 0;
			ResetPlayerWeapons(playerid);
			for (slot = 0; slot < elemsof (weapons); slot++)
			{
				GivePlayerWeapon(playerid, weapons[slot], ammo[slot]);
			}
		}
	}
	return ret;
}

That's almost the whole fix, and quite a mechanical conversion from the PAWN version (this was slightly more modified as I replaced the 2d array with two arrays instead, but that's a minor point) (it is also missing break in the switch because it isn't needed when there is no fall-though, but probably should be there).

Y-Less avatar Jul 25 '17 17:07 Y-Less

You don't even need to mess about with AMX * amx and cell * params for the most part - I've set up a load of macros to do all the conversions for you (maybe they should be made more widely available, it probably makes writing natives quite easy as well).

Y-Less avatar Jul 25 '17 17:07 Y-Less

OK, I've got a pretty stable plugin base (at least on Windows for now). Do people want to start porting the fixes and testing it? A lot of them will be quite mechanical conversions when they are self-contained. I'm not sure what the best way to do player loops is yet though, nor callbacks - but they are all in there. Hooking natives is trivial, as is adding new ones now - no messing about with arrays, addresses, and registration.

Y-Less avatar Jul 29 '17 12:07 Y-Less

So is this gonna also replace YSF and SKY? I'm voting for it. Especially while Kurta is available to help for the transition. rt-2

rt-2 avatar Jul 29 '17 12:07 rt-2

I'm not sure how much of an overlap there is currently (or should be - but that doesn't mean none). They have traditionally had slightly different roles, fixes was just purely that with new functions only added very very rarely, YSF and SKY were more feature-based.

So I don't know. I can mention it to @oscar-broman (Slice) as well.

Y-Less avatar Jul 29 '17 13:07 Y-Less

@Y-Less Since Windows is working, I can help making the plugin stable for Linux.

SysadminJeroen avatar Jul 29 '17 13:07 SysadminJeroen

@Y-Less what about memory hacking in this plugin? Is it allowed? Because some bugs easier to fix with some memory hacking magic.

ziggi avatar Jul 29 '17 13:07 ziggi

About iterating players, see std::set:

std::set< int > FIXES_Players;

bool OnPlayerConnect( playerid )
{
    FIXES_Players.insert( playerid );
    return 1;
}

bool OnPlayerDisonnect( playerid, reason )
{
    FIXES_Players.erase( playerid );
    return 1;
}

I don't think that this plugin uses C++11 to be able to use auto and range loop:

for( auto i: FIXES_Players )
{
// i = playerid
// i is directly an int, not a pointer !
}

so you should probably add a definition like

#define for_set(container,type,variable) for (std::set<type>::iterator variable = container.begin(); variable != container.end(); ++variable)

to be able to use it like

for_set( FIXES_Players, int, i )
{
// *i = playerid
}

This plugin should definitely be only to provide fixes and additional functions like GetPlayerDialog. It would be the best to combine YSF with SKY, but not with the FIXES plugin too.

Also, as I said before, I don't use YSF mainly because of the memory hacking. Using memory hacking in this plugin too would suck pretty much. I, for example, don't want to wait for someone to update the addresses when a new SA-MP version is released (let's just hope) to be able to upgrade my server safely. Using memory hacking as @ziggi said could give us some benefits, for example getting the vehicle colors for GetVehicleColor directly from the memory, but I don't think that it is worth it. Updating the addresses is pretty bad.

IstuntmanI avatar Jul 29 '17 13:07 IstuntmanI

I don't think we need memory hacking at all any more - even the hooks are done with generic methods now, thanks to advances in other libraries (specifically subhook by @Zeex ).

Y-Less avatar Jul 29 '17 13:07 Y-Less

Jeron52: That would be great! There is one issue I know of, but it is not integral to this plugin iteslf, and I think I know how to fix it (I'll probably move the code involved in to a generic library for other plugins to use).

Y-Less avatar Jul 29 '17 13:07 Y-Less

I'm also currently using several customised versions of third-party libraries, but most of the changes I'm pushing upstream. I'd like to be able to build a single binary and not have to have separate dlls for samp-log-core and sampgdk, but for now it is working as-is, and will continue to do so at least for testing and development.

Y-Less avatar Jul 29 '17 13:07 Y-Less

@Y-Less Just tell me what to test and I'll get on it.

I'm using CentOS 6, however I am able to test it on CentOS 7 or any other distro if necessary.

SysadminJeroen avatar Jul 29 '17 13:07 SysadminJeroen

Currently I have this line in the code:

__pragma(comment(linker, "/EXPORT:_" #func "=_NATIVE_" #func));

That is so that other plugins can import the natives from this one directly and call them, instead of having to go through the AMX. That takes an internal function with the prefix NATIVE_ and exports it as just the native's name (with a _ prefix for _cdecl). But you can't export them with a different name like that as far as I can see in Linux. However, I'm going to change how it is done anyway, so it will be fine...

Y-Less avatar Jul 29 '17 13:07 Y-Less

Also @IstuntmanI I'm already using range-based for loops and several other C++?? features (don't know their exact spec dates, but recent). As long as it builds for older OSes, it doesn't matter which C++ version it is written in - it doesn't have to be built on those older ones.

Y-Less avatar Jul 29 '17 14:07 Y-Less

Well I think I fixed the import on Linux problem (but I have not even tried to compile the code, so...)

Y-Less avatar Jul 29 '17 15:07 Y-Less

Also @Jeroen52 probably the biggest problem with Linux right now (since I think I solved the other one) is just compiling - there is no makefile or anything.

Y-Less avatar Jul 29 '17 17:07 Y-Less

@Y-Less A makefile should be easy to make if the plugin is easy to compile.

SysadminJeroen avatar Jul 29 '17 20:07 SysadminJeroen

Just saw someone linked me to this post:

http://forum.sa-mp.com/showthread.php?p=1479967#post1479967

Check the date...

Y-Less avatar Jul 30 '17 16:07 Y-Less