openmultiplayer / omp-stdlib

Pawn includes
Mozilla Public License 2.0
31 stars 17 forks source link

Redundant definitions from YSF and fixes.inc #27

Open NexiusTailer opened 6 months ago

NexiusTailer commented 6 months ago

All those definitions implies that omp functions and general behaviour is completely the same as in those samp libraries: https://github.com/openmultiplayer/omp-stdlib/blob/05ba3402eb1e235dc8a448c1054924db3d17c3c4/_open_mp.inc#L510-L513 but it's not, so they're cannot be used as a marker it's fully interchangeable.

The problems it produces are in two simple examples:

User A used YSF plugin for a couple of natives which is still not implemented in omp (for example, GetLocalIP or IsBuildingRemovedForPlayer). He moved to omp server, and the first output he sees in the console when compiling a script is that YSF was skipped from including, like all its functions already built-in in the server and declared in libs. But at the same time, he sees several errors that there is no any functions like GetLocalIP or IsBuildingRemovedForPlayer. Wait, isn't YSF was skipped just by the reason not to catch these things? A proper way to notify user A without confusing like above is already inside the omp server itself: when he tries to load YSF, which is memhack plugin and half of the functions is in omp server, he receives a lot more clear and detailed message about it:

It requires memory hacking to run and is therefore broken on open.mp, we already added many built-in features from YSF to open.mp and the rest are coming

Meanwhile, user B used fixes.inc under samp server. Before the move to omp server, he can disable broken stuff which obviously creates more harm than good (for example, changing behaviour of ClearAnimations inside vehicle without a reason). At the same time, before, he can enable something that were considered questionable, but just for his gamemode, as it fits his needs (for example, desync player and ignore his chat inputs while any dialog shown). Anything from his settings can no longer be done, so it's again not really interchangeable. Moreover, unlike using YSF plugin there are no real obstacles to still include fixes under omp server just to selectively use only the necessary fixes (which are not included in the omp server and present in fixes.inc).

So, I suggest to omit these defines as they anyway don't fulfill the original idea and only bring confusion.

Y-Less commented 6 months ago

That's true, and I agree a better solution is needed, but it is a bit more complicated than just removing them. Currently they will prevent the inclusion of those files entirely, which will as you say give errors that those functions don't exist. On the other hand not preventing their inclusion will give a large number of duplicate function errors. Neither case is ideal, but I'd say the former is more useful to end-users; though the latter is probably slightly more possible to deal with (with effort). A third option is to somehow require including YSF/fixes.inc first and deal with the conflicts in here instead, which puts the code entirely with us.

I'll have a look at the other includes and see what I can come up with.