oscar-broman / samp-weapon-config

A more consistent and responsive damage system with many new features
Apache License 2.0
92 stars 85 forks source link

Add support for non-y_iterate lib gamemode. #166

Closed zile42O closed 4 years ago

zile42O commented 4 years ago

This is only for people who not using the YSI packages, like me so i made this easy, the scripters before including the Weapon Config can set with code:

define WC_YSI_PACKAGE false, and disable Foreach iterate function and '#error the YSI library is required, get it here: github.com/pawn-lang/YSI-Includes' error.

The YSI Libary is by default set to enabled (true). Y-Less i respect your work this is not personal, but someone don't want to include all package via 1 include. Thats it, thanks.

ADRFranklin commented 4 years ago

Sorry we had code similar to this before, but in favour of just using y_iterate, there is no need to do this anymore. We insist that people use y_iterate.

Also this way of doing it is wrong, and adds more definitions then there needs to be.

zile42O commented 4 years ago

I don't understand who are you or someone to insist to i use y_iterate, i did this because many ppl contacted me, and i spent time for this, so if people don't use ysi package, for example here foreach function then that people not need this include, i think is better to have something like this to scripters have choice. Also this way of doing it is wrong, and adds more definitions then there needs to be. I am not sure what is wrong? I did with macro check only 5 lines per foreach code, this way is better then all code transform in macro if else end, if you understand me what i want to say. And and adds more definitions then there needs to be. this add 1 definition who scripters who have ysi package don't need to add before including he can ignore it, or if don't want in this case foreach then do:

define WC_YSI_PACKAGE false

include < weapon-config >

So what is problem, your mind to using only 'foreach' or choices to other scripters?! Thanks

ADRFranklin commented 4 years ago

The problem with your code is it requires the user to remember to define a variable like WC_YSI_PACKAGE instead of just checking if they have y_iterate included _INC_y_iterate.

Second, I don't know where you get off, telling me how I should maintain this library? I have had no one complain to me about this, so many people asked for y_iterate support that I deemed it un-needed to support both and instead just use y_iterate.

Why not instead of telling me how to maintain this library, you go and instead make an issue asking for me to add back support for the original version.

zile42O commented 4 years ago

Yes man you are right i made small mistake for that, that i can fix, i understand you but understand you me, i not using foreach function this is only example, i don't want it, maybe someone more not using it like me, so to he need wait for you to u give it without that, isn't it better to make something to just turn off it - if someone want, and u don't need to be haughty u can just say, yo bro u make mistake for that or that, thats it, chill out man

ADRFranklin commented 4 years ago

@zile42O I barely understood that, but at no point in my previous messages was I not cool about it? I simply pointed out that we previously had code that supported it, but when I made an issue asking to continue support for it, everyone gave the thumbs up to continue without supporting it.

The reason for this, was because the additional compiler directives made the code silly to manage, and that is why it was removed. The code is cleaner and more manageable, and we take advantage of libraries that make the developer process easier.

I think you just misunderstood what I said and how I said it from the start, and you seem to be upset from your own misunderstanding. All you had to do was make an issue about this, and then I would of been informed of the situation. But instead you made a PR in which I clearly explained the reason as to why support for it was removed.

zile42O commented 4 years ago

So what is solution now, here i don't want to use that so what is solution to every time when this code is updated to i clone it, and update it for myself, and other people who contact me, or something else, man this is for me stupid do what you want, but when you working for globaly think about other cases, 50% people can say ye this is good keep going, the other half can say no i don't want it, and he will find solutions: 1. make edits 2. stop using this 3. something else. thats reason why i speak this is maybe better way to keep foreach and default loops *(in this case), the people can have choice. PS: i understand you, but u always will have 2 or 5% ppl like me who don't use it, thats reason why i am mad about this, you make me update the code manually every time, and i don't care for all people who give 👍 i m here for the majority of others who fucking don't want manually make changes every time. Thanks for your response, i will wait now your response when u answer you can close it.

ADRFranklin commented 4 years ago

@zile42O I don't think you understood a single thing I said at all. For one I never said I wouldn't accept your PR, and I never said that adding back support is a bad idea. I simply said that when we did a vote to have support for it removed, you didn't voice your opinions then, and you should of if you wanted support to have stayed.

I can't accept your current implementation, because it's adding extra work where there doesn't need to be. I can't accept WC_YSI_PACKAGE having to be defined to use the library.

I would prefer you checked if y_iterate was included by the define I mentioned before, if you can't do that, then I will close this PR and I will make the change myself.

zile42O commented 4 years ago

Bro check code i think thats it, i did it, and look i said 'if' developer don't want to use YSI libary he just need to do:

define WC_YSI_PACKAGE false - he set is on false, if he don't do that automaticly YSI is enabled

include < weapon-config >

and i did commit new for that check it @zile42O Update weapon-config.inc

I think thats that what u said, maybe i didn't understand you good with I can't accept your current implementation, because it's adding extra work where there doesn't need to be. I can't accept WC_YSI_PACKAGE having to be defined to use the library. myb you want without that, but that is only for not using ysi lib xD?

zile42O commented 4 years ago

i can do if is included before weapon config then set to true, if is not then to false if you want that?

ADRFranklin commented 4 years ago

@zile42O Okay so you haven't understood what I asked you to fix. So let me explain it a little better.

the YSI library called y_iterate defines this _INC_y_iterate when it is included. This means all you have to do to check if y_iterate is included is to do

#if defined _inc_y_iterate
    foreach(...) { ... }
#else
   for(...) { ... }
#endif

that is all that is needed to check if YSI is included or not. As for the part that tries to include y_iterate you just need to remove this code from the statement.

    #if !defined _inc_y_iterate
        #error #error the YSI library is required, get it here: github.com/pawn-lang/YSI-Includes
    #endif

that is the only changes you need to make to make this work.

zile42O commented 4 years ago

Yeah you are right i will do that.

zile42O commented 4 years ago

But question one, you need to be for example:

#if defined _inc_y_iterate
foreach (new i : Player) {  
somecode;
}
#else 
for (new i = GetPlayerPoolSize(); i != -1; i--) {
somecode;
}
#endif  

or

#if defined _inc_y_iterate
foreach (new i : Player) {  
#else 
for (new i = GetPlayerPoolSize(); i != -1; i--) {
#endif
somecode;
}
ADRFranklin commented 4 years ago

The second one is the more preferred style.

zile42O commented 4 years ago

Check it?

zile42O commented 4 years ago

Yeah i think thats it, only 1 problem is why i was think is better to add on my way with define before include in cases if compiler lib have YSI but developer's current script not using it, but nvm, i was think to do on your way, nvm xd. myb is spaces in if else lines but idk why i didn't input it but i see it on github, i will make changes if you see it..