Print

Add to Reputation:  

What do you think of this post?

I agree I disagree  
Comment:

[JohnRDVSMarston]

I installed once again the game and played a little bit of Arsenal. I won the first round and, to savor the victory, I suicided (in-game). Unfortunately my PC is crap, so it took a bit to die, even so that I only suicided in the second round.

But, there, things came a little different: I killed just one person and I leveled-up (it means, instead of two-kill level-up, it was one-kill LU). Unfortunately, I died later, so I don't know if I was gonna get one-kill level-ups forever or just in the first weapon (probably because the game thought that I was using the last weapon, the knife).

I was playing in the "Bonds Only" server... Something on the line, I don't remember. The point is: It was vanilla (Ice-ice,baby) Arsenal, not modified.

[Entropy-Soldier]

Shoot, minor oversight on my part.  The fix will be in the next release.

It's actually a bit more involved than a one line fix (if you want to keep the intended behavior instead of just dropping to 0 kills every level downgrade), otherwise I'd just post it here.  If anyone wants to run the patched version let me know and I'll upload it somewhere.  You'll get the (MOD) tag next to the gamemode name but at least one of the optimal opening strategies won't be to instantly kill yourself.

[JohnRDVSMarston]

Well, I can help you. But you will have to wait a bit for me to try it out (like, 'till 19:00 here, don't know about there, but I had to add 7-8 hours on the forum's clock).

[Entropy-Soldier]

Well, I mean, I know it works.  I just wanted to see if there's actually any interested server owners before posting it, as a service to anyone who wants this bugfix ahead of time.

[null]

E-S, I've already submitted a fix for this and already been pulled: https://github.com/goldeneye-source/ges-python/pull/6

[null]

I installed once again the game and played a little bit of Arsenal. I won the first round and, to savor the victory, I suicided (in-game). Unfortunately my PC is crap, so it took a bit to die, even so that I only suicided in the second round.

But, there, things came a little different: I killed just one person and I leveled-up (it means, instead of two-kill level-up, it was one-kill LU). Unfortunately, I died later, so I don't know if I was gonna get one-kill level-ups forever or just in the first weapon (probably because the game thought that I was using the last weapon, the knife).

I was playing in the "Bonds Only" server... Something on the line, I don't remember. The point is: It was vanilla (Ice-ice,baby) Arsenal, not modified.

Hey thanks for reporting, I've spent hours working on this. I'm also glad its going to be in the new patch.

The problem was "KillsPerPlayer" got mixed up with "self.pltracker[player][TR_LEVELKILLS]"  in the calculation for suicide, so instead of removing one kill from total player kills ("self.pltracker[player][TR_LEVELKILLS]"), it removed one Kill from total kills to level ("KillsPerPlayer"), so no matter what the server owner sets its always one suicide and then the next kill to level OR KillsPerPlayer - 1 and then the next kill to level, which is the bug.

The reason this only works on the very first level is because the function "ar_IncrementLevel" does not equal levels greater than zero "(self.pltracker[player][TR_LEVEL] + amt > 0)"    ---> ("self.pltracker[player][TR_LEVEL] == 1" and "amt == -1")  1-1 equals zero, but not greater than zero so the ifelse statement resets the Kills and Levels of the player.  The "ar_IncrementLevel" function is before the kills calculation above so it pretty much does nothing here. If  "self.pltracker[player][TR_LEVEL]" equal 2 then the problem is accounted for in the function.

There is a one line fix, seen here: https://github.com/goldeneye-source/ges-python/pull/6/files

[Entropy-Soldier]

Dang, how could I have missed that commit?

Sorry to say this but you're not replicating the intended functionality, which IS to use KillsPerPlayer.  The idea is that you don't lose an entire level and all your kills when you drop a level, you only lose however many kills you just crossed the threshold by.  The comment isn't entirely clear about this so I guess that's my fault but outside of that your change still makes no sense to me.


For some context, this function is only called to:
     reset the player's kills to 0, which doesn't execute this codeblock
     increment/decrement the kill counter by 1

In the second case, only decrementing the kill counter can ever lead to this code block being executed.  The relevant line is, of course:

Code: [Select]

    # Advance the given player's level kills by the given amount.
    def ar_IncrementKills( self, player, amt ):
        self.ar_SetKills( player, self.pltracker[player][TR_LEVELKILLS] + amt )

Your modified function:

Code: [Select]

    def ar_SetKills( self, player, kills ):
        if not player:
            return
        
        if ( kills >= self.KillsPerLevel ):
            self.ar_IncrementLevel( player, 1 )
            self.pltracker[player][TR_LEVELKILLS] = 0
        elif (kills < 0):
            self.ar_IncrementLevel( player, -1 )
            self.pltracker[player][TR_LEVELKILLS] = max(self.pltracker[player][TR_LEVELKILLS] + kills, 0) # Kills is negative here so we're using it to correct previous level killcount.
        else:
            self.pltracker[player][TR_LEVELKILLS] = kills # No level advancement, just complete the request as asked.
            msg = _( "#GES_GP_GUNGAME_KILLS", str(self.KillsPerLevel - kills) ) # We didn't increment a level which would have caused a level advancement message.
            GEUtil.HudMessage( player, msg, -1, 0.71, GEUtil.Color( 220, 220, 220, 255 ), 1.5, 2 ) # So give a killcount message instead.

Relevant codeblock:

Code: [Select]

        elif (kills < 0):
            self.ar_IncrementLevel( player, -1 )
            self.pltracker[player][TR_LEVELKILLS] = max(self.pltracker[player][TR_LEVELKILLS] + kills, 0) # Kills is negative here so we're using it to correct previous level killcount.

but keep in mind that the only function call that can ever trigger this is called with a parameter of "self.pltracker[player][TR_LEVELKILLS] + amt" so we can substitute that in for kills and get:

Code: [Select]

        elif (self.pltracker[player][TR_LEVELKILLS] + amt < 0):
            self.ar_IncrementLevel( player, -1 )
            self.pltracker[player][TR_LEVELKILLS] = max(self.pltracker[player][TR_LEVELKILLS] + self.pltracker[player][TR_LEVELKILLS] + amt, 0) # Kills is negative here so we're using it to correct previous level killcount.

So the line just became:

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] = max(self.pltracker[player][TR_LEVELKILLS] * 2 + amt, 0)

Keep in mind that self.pltracker[player][TR_LEVELKILLS] can NEVER be less than 0 so in the current program amt MUST be -1 (possible values are -1, 1) and self.pltracker[player][TR_LEVELKILLS] MUST be 0 (Only 0 - 1 < 0) for this to ever get called.  Thus the simplified version is just:

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] = max(0 * 2  - 1, 0)

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] = 0

So, easy fix if you wanted to always reset player kills to 0 on level downgrade.  However this would be horrible on servers that have like...5 kills per level or something so the intended behavior is to just require one more kill until the level can be regained, which is implemented mostly correct but overlooks that there might not always be a level to take off.  The solution is thus rather simple:

Code: [Select]

        elif (kills < 0):
            if self.ar_IncrementLevel( player, -1 ):    # If we can actually take off a level then:
                self.pltracker[player][TR_LEVELKILLS] = max(self.KillsPerLevel + kills, 0) # Kills is negative here so we subtract it from the max kills per level to give the player that many kills to get their level back.
            else
                self.pltracker[player][TR_LEVELKILLS] = 0

However this also requires ar_IncrementLevel to be adjusted to return true if a level can indeed be taken off, and false otherwise, so it's more than a 1 line fix.  Ultimately I spent more time on this post than I did on the actual fix, so maybe I overlooked something else or there's a better way, but I tested this and it seems to work well enough. 

Honestly I might overhaul arsenal to have a more seamless design for 5.1 to avoid any future confusion. The functions imply and poorly implement more functionality than they actually have, since I adapted previously existing code for a completely new design and then half-assed making it more versatile than needed.  Regardless, The fix I just detailed works in the event I don't get around to fixing up the rest of the code.


I would recommend becoming proficient with algebra if you want to approach problems like this, tools like variable substitution are very handy for contextualizing nested functions.  I've obviously got a lot to learn myself as I haven't been doing this for all that long in the grand scheme of things, but I have to say math comes in handy more often than most other things for me.  I'd also recommend making friends with fellow programmers to pick up more neat tricks, I'm sure there are plenty of other upcoming projects that would love to have your help and full attention.

[null]

Thanks E-S, for the fast response, but i think you went through code a little fast, i don't understand your fix, maybe i could see your complete fixed version?

In your solution i saw your if statement calls self.ar_IncrementLevel, but self.ar_IncrementLevel doesn't have a return ('return true' or 'return false') so it will always fail. You can change self.ar_IncrementLevel function as much as you like, but because the kill counter is set after that function is done the kill counter will still be set wrong.   

To explain my line change as simple as possible, if you as a player suicide, you only lose 1 KILL, and will NEVER lose a LEVEL, your kills just bottom out to zero if somehow the kills go negative, because the max() function will always set kills to the highest value which is zero in that scenario. 

Regarding the

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] = max(0 * 2  - 1, 0)

not sure where this calculation came from, but i didn't write that. The flaw with that is it's the same as writing:

Code: [Select]

(self.pltracker[player]  [TR_LEVELKILLS] = 0)

  every suicide drops all your kills back down to zero no matter how many kills you have.

[Entropy-Soldier]

However this also requires ar_IncrementLevel to be adjusted to return true if a level can indeed be taken off, and false otherwise, so it's more than a 1 line fix.

So if you want to implement that fix I just posted be sure to do that too.  I only included the relevant code block so I didn't have to bog down the post with another huge function that could be explained away in one sentence.  I'm taking things step-by-step, so stuff like:

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] = max(0 * 2  - 1, 0)

is to show specific values for inputs in the event that only one specific set of inputs can ever result in that code block being executed, which is the case here.

On that note, it seems like you've made a quintessential misunderstanding of this function.  ar_SetKills is to set the ABSOLUTE kill counter of the target player.  Calling it with a value of 0, for instance, puts them at 0 kills regardless of what value they were at before.  ar_IncrementKills is to increment/decrement the kill counter by the given amount, calling it with 0 will do nothing because it's just adding that value to the kill counter.  Calling ar_SetKills with a negative value does not mean the player suicided, it means we want to take off a kill when there are no more kills to take.  It's in the same vein as some subtraction algorithms, where you decrement the next most significant digit when you're subtracting more than the current one has.

On suicide, ar_IncrementKills is called with a value of -1:

Code: [Select]

    # Advance the given player's level kills by the given amount.
    def ar_IncrementKills( self, player, -1 ):
        self.ar_SetKills( player, self.pltracker[player][TR_LEVELKILLS] - 1 )

This, as you can see, calls ar_SetKills with a value of "self.pltracker[player][TR_LEVELKILLS] - 1".  So let's replace all instances of the variable "kills" in that function with "self.pltracker[player][TR_LEVELKILLS] - 1".  This is to show what ar_incrementKills is actually doing when called with a value of -1, which it will be in the case of a suicide.

Code: [Select]

    def ar_SetKills( self, player, self.pltracker[player][TR_LEVELKILLS] - 1 ):
        if not player:
            return
        
        if ( self.pltracker[player][TR_LEVELKILLS] - 1 >= self.KillsPerLevel ):
            self.ar_IncrementLevel( player, 1 )
            self.pltracker[player][TR_LEVELKILLS] = 0
        elif (self.pltracker[player][TR_LEVELKILLS] - 1 < 0):
            self.ar_IncrementLevel( player, -1 )
            self.pltracker[player][TR_LEVELKILLS] = max(self.pltracker[player][TR_LEVELKILLS] + self.pltracker[player][TR_LEVELKILLS] - 1, 0) # Kills is negative here so we're using it to correct previous level killcount.
        else:
            self.pltracker[player][TR_LEVELKILLS] = self.pltracker[player][TR_LEVELKILLS] - 1 # No level advancement, just complete the request as asked.
            msg = _( "#GES_GP_GUNGAME_KILLS", str(self.KillsPerLevel - self.pltracker[player][TR_LEVELKILLS] - 1) ) # We didn't increment a level which would have caused a level advancement message.
            GEUtil.HudMessage( player, msg, -1, 0.71, GEUtil.Color( 220, 220, 220, 255 ), 1.5, 2 ) # So give a killcount message instead.

Note that there are 3 cases to consider here:

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] - 1 >= self.KillsPerLevel

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] - 1 < 0

Code: [Select]

else:

aka

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] - 1 >= 0 && self.pltracker[player][TR_LEVELKILLS] - 1 < self.KillsPerLevel

With the important consideration that self.pltracker[player][TR_LEVELKILLS] is always >= 0 and < self.KillsPerLevel.

This case, then, can be eliminated right away since you can't subtract from something less that self.KillsPerLevel and end up greater than it. (unless you subtract so much you overflow the integer, which is obviously not the case here.)

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] - 1 >= self.KillsPerLevel

so our remaining two cases revolve around:

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] - 1 < 0

if self.pltracker[player][TR_LEVELKILLS] is >= 1, then this will not execute since any non-zero integers will result in a value of 0 or greater.  Thus the codeblock in question ONLY executes when a player kills themselves AND has 0 kills on the current level.  This results in the only values ever being passed to the code after this conditional statement being self.pltracker[player][TR_LEVELKILLS] = 0, and kills = -1.

Then my previous explanation applies.  Just plug in the values:

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] = max(self.pltracker[player][TR_LEVELKILLS]  + kills, 0)

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] = max(0 - 1, 0)

Code: [Select]

self.pltracker[player][TR_LEVELKILLS] = 0

I've actually tested your code just to be sure, and it behaves exactly as I described.  If you die with 0 kills, you lose a level and are downgraded to 0 kills on that level.  Not sure why you think that you can't lose a level since self.ar_IncrementLevel( player, -1 ) gets called regardless of what self.pltracker[player][TR_LEVELKILLS] is set to.  The only instance in which you don't lose a level is when you can't, which is level 1.  Obviously this is the one instance where the original code fails to behave as expected where yours captures the intended behavior, but given that the goal is not accomplished by either setup the third solution is required.


To restate the goal, let's use an example.  This server is running 3 a kills per level setup.  Each kill is represented with a number and each level is represented by set of numbers separated by a |.  The first 3 levels are thus:

012|345|678|....


If we have 4 kills and suicide, we downgrade to 3 kills but stay on the same level.  Thus we just need 1 kill to regain our previous standing which is more or less the standard across all gamemodes.

If we have 3 kills and suicide, however, we should still go down just one kill, to kill 2.  The only difference is that we also go down to the previous level in the process.  The standard is maintained by the fact that just one kill will put you back where you were.  With your fix, suicide after kill 3 will drop you to kill 0, which is not the intended behavior.


Anyway, there's pretty much nothing else to explain here, so if you still disagree with me and can't implement it yourself, I'll provide my entire set of modifications so you can verify that they behave as desired.  I figure that you at least deserved the full explanation as to why your changes won't be included, but anything more than this goes outside my ability to explain.  I guess I could be wrong, in which case no amount of explanation would be sufficient to explain this properly, but given that I've tested both versions of the code and did a pseudo mathematical proof as to why they behave as they do I can't think of a way that's possible.

[null]

Thanks E-S for going through all of that and explaining, sorry it took so long to respond. I know because of all the bad bloody lately how teeth clenching it must be to deal with my pain in the arse, i appreciate you going through the extra explanation none the less.

So yep, you're right, i went back over the code again, and my modification was way off, it did break suicide while on zero kills just as you said. I played around with it a bit and i think i got your fix right?  https://github.com/Jnull/ges-python/commit/e110548cae7a0982c8ca032dd9c21297fd593a22  (If that's not right I'll just wait for the patch or whichever)

Thanks for the recommendation on Algebra and other projects and stuff, I'll definitely consider that.

[Entropy-Soldier]

Yep, you got it!  It's ultimately a pretty simple fix, but I'll probably be recoding that part of arsenal so it's a bit easier for people to follow.  I threw it together super fast and made use of all the code already present from previous versions, which was perhaps not the right way to go about it since it did make the interaction between levels and kills pretty convoluted at certain points.

Anyway the biggest piece of advice I have to avoid this kind of situation is to thoroughly test everything to ensure that it captures the intended functionality before standing behind it.  A fix might stop one particular issue, but it can easily plug one hole to open several more if you're not careful and don't check everything.  I was completely certain of what I was saying and -still- did a thorough test of both setups before writing my post.


Anyway I'm happy to help, and I appreciate that you made this commit with the intention of helping us, but it's very hard to justify doing too much for you since you're kind of actively stabbing me in the side right now with impersonating the dev team and all that.  People need help with this stuff all the time, so it's really no issue for me to help out, but I do admit that I was questioning why I was being nice about it given the circumstances.  I can't really afford to do this kind of thing too often or it gives people the idea that I like to help people who actively work against my responsibilities.  Obviously I would like to avoid setting that precedent since every minute I spend dealing with people causing trouble is one I'm not spending on something productive.  Given that, consider this a one time deal unless you seriously turn things around.

[null]

Your help is much honored and i give you respect for that, maybe a re-code would good, but now i think i hate arsenal and team arsenal because it takes away from the original game, but that's another topic. If i do more coding, I'll try to test things more thoroughly next time for sure, like you said we all have much to learn right?

On the other stuff please don't be too hasty, i removed all Weapon Skins, Dev Tags on the day you locked the thread, nobody is impersonating anyone. Not that it matters now though, i originally did this for the achievements because hardly any of the Dev's play, but that doesn't excuse that i could have probably privately disclosed in the beginning before activating those features.

I don't mean to beat a dead horse, so i won't. Thanks again, and i hope to see you in-game again sometime.

[Entropy-Soldier]

Wait, you did?  Why did you not tell us?  I mean, really, with a sincere apology and gesture of goodwill you'd be mostly back in good standing.  I'm mostly just interested in making sure a few specific rules don't get violated, and that the position of the dev team on them is firm and clear.  If I fail to do this and a precedent is set, everyone will do what they want and that just brings the whole game down.  Obviously I don't want to fight with anyone if they're willing to respect the rules and not cause trouble, so showing that you're no longer doing that is a pretty big deal.

My intel suggested you were still doing the skins thing fairly recently, and since I had no reason to think otherwise I just assumed you kept up everything else too.  You did kind of dig a huge hole for yourself when you went down guns blazing in the last thread, so it's not quite as simple as "Oh, you were serious?  I'll stop now!", but if you still want to run servers and put in the effort to get soupcan and the rest of the team to forgive you then I don't have any reason to blacklist your servers anymore.  It might not be easy, but if you work to undo the damage then I think that's a fair deal.  Just don't let me catch you doing any of this again.  If you're not sure about something feel free to ask me before you do it.


Outside of that, just respect boundaries and the possible effects your actions will have on others and there won't be any problems between us.  I was mostly just booty-bothered because I thought you were doubling down.  I'm trying to play at least a little bit these days so maybe we will get a few friendly games with eachother!

[null]

Well i went back to reply to the topic and it was closed, you did assume i wasn't going to follow your directions and i think that's why you locked it. I've already taken steps so i don't have to worry about being banned or not, so that wasn't my concern, but like i said I'm not enabling the functionality or planning too.

Turok (XGII) got banned for forums for something i did, with all the conclusions flying around there might have been mistakes on both of our parts, but it was at least 50/50 on all accounts, maybe 51/49 but i wont say which side. You have to admit that in some degree, unlocking skins is encouraged, but i understand the dev tag piece and impersonation is bad.

I would just like to see more dev's on the front lines, which would have probably alerted you too it from the beginning, instead it just so happens that someone just asked an innocent question, i don't think Turok was trying to antagonize anyone when he first posted, which is the stance the first dev's replied with.

[Entropy-Soldier]

The blacklist system is entirely implemented on the client, so if you've got a way around that from the server's perspective I'd love to know because it's moreso a tool that we need in general and not focused on you specifically.


And I mean like, yeah, I locked the topic because I said you had one more reply and then that was it.  It didn't seem like you were going to change your mind, and you already rightfully pointed out it should have been locked before, so I figured it was in the interests of both parties to just have it over and done with.  You're always free to continue the discussion on discord/steam/PM after the topic gets locked, locking the topic has more to do with not making us both look dumb by running around in circles in public.

Anyway if you really want to discuss XGII then you're free to add me on steam/discord about it.  You don't need to feel responsible for his ban, he made it pretty clear that after we removed his one brightskins topic that he was determined to cause trouble over it.  Our policy is usually to temp ban people for a few days to give them a chance to think things over and cool off, but it seemed like he was jumping right back into it.  Mostly everyone wanted him banned before that whole topic even happened, but after another round of him throwing insults it was obvious things weren't going to get better.  We were obviously wrong about him modding in dev tags, but that wasn't even a minor contributor to why he was banned.  It was the spark that set the whole thing off, but don't hold yourself responsible for that.  If it wasn't that it was going to be something else.

Anyway if you want to continue this discussion just add me on discord, since long responses are not the best way to discuss this kind of thing since it's hard to address any individual topic completely.  But I will say that XGII can come back to the forums if he can make a serious gesture of apology to everyone he's attacked and gives us good reason to suspect he won't cause any more trouble.  It's going to have to be some real work on his part though because he did upset a lot of people, and it's not fair to the rest of the team if I just start pardoning people and letting them hang around again to the ire of everyone they've upset.  Banning isn't really a punishment so much as a last resort to keep the forums civil.  If he can mend some fences and won't start attacking us the moment we disagree with him then there's no reason to keep him banned.  It's just that...he might be a lot happier in another community and dwelling on this one might not do him any favors.  Obviously the more people in our community the better, but I want them to all enjoy or at least tolerate eachother's company and the environment we've created.  If that's difficult for someone then I don't want to ask them to suffer every second they're here just to keep the peace.  It shows and it's not good for anyone.