pawn-lang / sa-mp-fixes

Includes and plugins to fix various issues in the SA:MP server that can be fixed externally, leaving the devs time for other things.
137 stars 57 forks source link

GetVehicleInterior, GetPlayerSkillLevel, GetPlayerAttachedObject #71

Closed WoutProvost closed 6 years ago

WoutProvost commented 7 years ago

In the same way as GetPlayerWeather, GetPlayerWorldBounds, ... were added (even though they also exist in YSF, but it fits this fixes include more).

There are probably some others ... However, they have been added to YSF (see the wiki). So, not only is this an issue to add those functions, but about the overlap between YSF and this include.

Y-Less commented 7 years ago

If they are already defined somewhere, and adding them to this include is both slightly out of scope (though we do tend to define completing get/set pairs as fixes) and will cause issues, surely the best answer is not to add them?

WoutProvost commented 7 years ago

I do think that adding the getters fits this include more than it does YSF t.b.h. So I am in favour of removing those from YSF and adding them here, however that's probably not going to happen. Therefore I don't see the point in adding fixes that already exits in YSF, as done with GetPlayerWeather and GetPlayerWorldBounds. But if we do decide on keeping them here as well, then don't just pick and choose, but add all of the missing getters.

WoutProvost commented 7 years ago

Just found out that GetPlayerWeather and GetPlayerWorldBounds give the following errors when YSF and Fixes are used in the same script: Errors

So either YSF or Fixes should remove the getters they have in common. The top of my script is as follows:

#include <a_samp>
#include <fixes>
//Other includes
#include <YSF>
ziggi commented 7 years ago

Just define this before fixes including:

#define FIX_GetPlayerWeather 0
#define FIX_GetPlayerWorldBounds 0

or just this:

#define FIX_Natives 0
WoutProvost commented 7 years ago

Hmm ok, hadn't thought of that. My original point still stands though.

ziggi commented 7 years ago

Also GetSpawnInfo should be added too.

Y-Less commented 7 years ago

I will say I'm not too keen on adding many more Get functions, even those that "should" exist. The ones already added tend to be ones where we store the information internally anyway for the purposes of other fixes. In those cases exposing the data doesn't really cost anything. Adding something like GetSpawnInfo would require a whole new set of hooks and variables just to make it work from SetSpawnInfo etc.

WoutProvost commented 6 years ago

This issue can be closed then.