-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Regarding **abuse** of OnPlayerConnect and OnPlayerDisconnect for scripts #807
Comments
+1 |
5 similar comments
+1 |
+1 |
+1 |
+1 |
+1 |
+1. Not a fan of "hacky" workarounds |
+1 What about |
+1. |
I always support any innovations. But I think this is a strange proposal. What's the point of these two callbacks? I have never encountered such a problem and neither have everyone I know. Yes, this is no reason not to notice it, but adding these new callbacks is strange for all the reasons. Although I'm not advocating not adding this, so +-1 ( 👍 👎 ). |
+1 sir I need hacktoberfest shirt |
+1 |
+1 |
+1 |
+1 - I'm a bit with @shierru here, I don't necessarily think the new CBs are useful ; OTOH at least these won't break my scripts if I don't use them, so it's still a MASSIVE improvement over what we've got going on right now. :-) |
+1 rhis is a great suggestion and will be very useful |
I'll just leave here what I found out in 2017 and wrote about in 2020, so yeah I don't like this abuse eitherReferences:
|
@WoutProvost same. Reported it here when it was in beta. |
+1 |
idk this whole codebase is a mess. |
fix it |
Reverted. Not happening. |
Reopening, we are not finished with this.
Let's not forget how many things you have broken with Fixes component and I've been trying to compromise all this times. |
Why does this need to be a part of the pawn component? Can't fixes extend pawn and do it there? If you don't want it don't use fixes. No clue why this needs to be in an expected part of the server. |
Wow this issue is being reopened again, i have heard this from my friend and got here. And also, That is what im going to said about this issue, +1 if this will be removed in the future. |
Again, reopening it, leaving it open for discussion to see if anyone can make sense out of it first before closing it (they can't) |
I would like know what you mean by this exactly. What "compromises"? All I've seen is people constantly fighting against features and trying to get them removed (like the fixes component currently being disabled by default, which it shouldn't be). It does feel like your definition of a "compromise" is "lets just half remove this feature". I even offered a compromise on this issue, i.e. that of having a flag to disable it, and that has been ignored. |
Just don't call it fully backwards compatible and go break shit. While fixes may or may not be a good thing if you start from scratch doesn't mean it needs to be enforced to all users especially if people recognize it as drop in replacement server. Dunno what to say you had your chance with the old macro hell of base, if it breaks shit because people don't really expect the way a fix changes behavior in their script or increases the load for a "single threaded app" then it doesn't solve shit. And I'm pretty sure there are enough people that encountered such issues due to unexpected behaviors. |
After almost 4 months of this issue being created and kept open, finally you decided to talk. And before that not only you decided to not speak out your opinion on this, but you also pushed into master to revert a change clearly not even a single thinks "makes sense", it's not about wanting anymore, it's about making sense now I'd be fully reading your messages now but I'm done with this. I am not going to waste my time on someone who still has around 60 messages from me in different places to answer, and one is literally the one you quoted from. always ignoring the important matter and nitpicking from my wall of text. If you have anything against this, please talk about and while you're doing that, read what others say as well. At this point I'm convinced you never read anyone's messages, not even your team mates. due to lack of any type of proper response. And if this happens again, I guess it means you are unable to provide enough reasons for feature (not a fix in any way) you're asking for. |
Resolved |
Just like what title says, this is an abuse, we are not limited nor restricted to stick with those events. If it was working back then in one 3rd party library, not used widely enough to be our base of opinions and implementations
Inconsistency & making zero sense:
"Call" OnPlayerDisconnect for connected players?!
"Call" OnPlayerConnect for already connected players?
The amount of people reaching out to me because this breaks their single filterscript map file, or an admin script, or anything else is a lot, and the fact that it makes absolutely zero sense (player is already CONNECTED!).
Does player leave your server when you unload a filterscript? No!
Does player just connect to your server when you try to load your filterscript? No!
So why are we making this breakable and unmanageable change to mess with people's scripts? How is it possible to distinguish whether player has just connected to your server, and it's not just a simple filterscript reload/load?
How is any of this related to PLAYER'S CONNECTION? do we do anything on network level when a filterscript gets loaded? I sure hope not!
So what's the solution?
Create two new callbacks/events for that. you load a script, a new event dedicated to that cause gets called, like
OnPlayerJoinScript
, orOnScriptLoadForPlayer
?Then keep
OnPlayerDisconnect
andOnPlayerConnect
to what they are meant to be used for, to what they are created for, to what they are meant to, the meaning!Final words, most important ones
This is an issue made for community, I'm asking anyone seeing this to give their opinion on this, consider this a community poll.
If you have nothing to say, use numbers, use + or -, just say
+1
or-1
.The text was updated successfully, but these errors were encountered: