Variable is not None even if there's no assignment

[PrinceOfFunky]

Here I'm again, this time with this function:

Code: Select all

event Timer() {
	local Actor actor;
	local LocationInfo nearestlocInfo, locInfo, newLocInfo;
	
	foreach AllActors(class'Actor', actor)
		if (trackActor(actor) || actor.IsA('Bot') || actor.IsA('PlayerPawn')) {
			//FIXME
			Level.Game.BroadcastMessage(0@actor);
			if (actor.IsA('Pawn') && (Pawn(actor).PlayerReplicationInfo != None) && Pawn(actor).PlayerReplicationInfo.bIsSpectator && Pawn(actor).PlayerReplicationInfo.bWaitingPlayer) {
				//If the actor is a spectating pawn do nothing.
			} else {
				foreach actor.RadiusActors(class'LocationInfo', locInfo, minDist - actor.CollisionRadius, actor.Location) {
					//FIXME
					Level.Game.BroadcastMessage(1@actor);
					if (locInfo.isGoodCandidate(actor.class))
						if ((nearestlocInfo == None) || VSize(locInfo.Location - actor.Location) < VSize(nearestlocInfo.Location - actor.Location))
							if (FastTrace(actor.Location, locInfo.Location))
								nearestlocInfo = locInfo;
				}
				
				//FIXME
				Level.Game.BroadcastMessage(2@actor@nearestlocInfo);
				
				if (nearestlocInfo == None) {
					newLocInfo = Spawn(class'LocationInfo', Self,, actor.Location);
					newLocInfo.actorClass = actor.class;
					newLocInfo.attentionLevel++;
				} else {
					nearestlocInfo.attentionLevel++;
				}
			}
		}
}

trackActor() just checks if the actor should be included in the following body, LocationInfo.isGoodCandidate() checks if a location info is compatible with an actor(it works fine), it is called only on LocationInfo's around the actor which its distance is 100 maximum.
The log 0 is printed if the actor has been included.
The log 1 is printed for each LocationInfo around the actor(within a radius of 100uu in my case).
The log 2 prints the nearestLocInfo, None if there was no any good LocationInfo candidate.

I start the match with one player(me) only, since there are no LocationInfo at the beginning, logs start by printing 0 and then 2, showing the nearestLocInfo as None, then they start printing 0, 1 and 2 since at least one LocationInfo has been spawned.
The problem comes when I spawn my cavy(a Nali of course), I don't move anymore to see what happens and I hurt the Nali so that he moves away, when he does it the logs keep printing 0, 2.
I mean, it never prints 1 and I could handle a bug like that but the problem is that, when it prints 2 the nearestLocInfo is not None!(The printed one is the same used by the player).
So, how does it come that nearestLocInfo is not None if the logs show 0, 2 only, without going through the log 1?
If nearestLocInfo is set, then the log 1 should be printed, unless I'm blind or whatever...
(Also, I checked and nearestLocInfo is nowhere else than in this function)

[MrLoathsome]

derp

[Feralidragon]

Are you absolutely sure you're only seeing 0s and 2s ?
Because this looks like one of those classical programming mistakes.

Essentially, I think you're actually having 1s too at least in one of the iterations, and since you're not resetting the nearestLocInfo reference to None at the end of each iteration, the next iteration will still have it set to whichever reference it was set previously.
But then, as far as I can tell, you don't want to reset it since you're incrementing nearestlocInfo.attentionLevel there, so your code is just plain wrong I'm afraid, and you will have to move things around there and do it in some other way.

Also a remark if you allow me, I suggest you to learn De Morgan's laws ( https://en.wikipedia.org/wiki/De_Morgan%27s_laws ), because this:

Code: Select all

if (actor.IsA('Pawn') && (Pawn(actor).PlayerReplicationInfo != None) && Pawn(actor).PlayerReplicationInfo.bIsSpectator && Pawn(actor).PlayerReplicationInfo.bWaitingPlayer) {
    //If the actor is a spectating pawn do nothing.
} else {
    ...
}

could as easily be replaced by this:

Code: Select all

if (!actor.IsA('Pawn') || Pawn(actor).PlayerReplicationInfo == None || !Pawn(actor).PlayerReplicationInfo.bIsSpectator || !Pawn(actor).PlayerReplicationInfo.bWaitingPlayer) {
    ...
}

or even if you don't know them:

Code: Select all

isSpectator = actor.IsA('Pawn') && (Pawn(actor).PlayerReplicationInfo != None) && Pawn(actor).PlayerReplicationInfo.bIsSpectator && Pawn(actor).PlayerReplicationInfo.bWaitingPlayer;
if (!isSpectator) {
    ...
}

"actor" is not a name I would use for any variable.
Change the name of that and try it again...

Actually, there's absolutely no problem in doing that, and unlike many may argue, it's not even a bad practice in doing so.
I know many will argue against it, and years ago I would to, but fact is that often, even in other languages, there's no better name to call an instance of Something as "something" itself, albeit is generally best done by having the classes in PascalCase while the vars as camelCase or snake_case.

Just think a bit about it: if he called it just "A" as it's often done, and if that presented a problem, no one would have any guarantees that it would actually work every time either, even because some other mod could very well have a class also called "A", you see?

The contexts and scopes of things are what matter the most, and there's no other context that "Actor" is deemed a class reference other than in var type declarations and when using as "Class'...'" or "class<...>", so "actor" is unambiguously always something else in every other case when used this way.
Also, although UScript is case-insensitive, it was a good idea from him to call it "actor" with lowercase rather than "Actor", to make the reference even more plain clear.
Epic does something similar in their code as well, mainly on render calls ("canvas Canvas"), although they did the other way around for some reason (lowercase for the class, and PascalCase for the parameter).

Of course, there are reserved names and the like, and those can never be used, but generally that's no problem because usually they're also reserved from being used as class names anyway.

[Barbie]

Also for debugging I'd simplify that code as much as possible: use constants instead of variables and function calls, leave out unneeded code, add more log messages, don't put it into a timer but trigger that.
If you are able to melt that down to a dozen lines, you could make and publish a test map. Compiling and running that code it via brain may produce headache and dizziness.

[PrinceOfFunky]

Read the EDITs .o.

Also a remark if you allow me, I suggest you to learn De Morgan's laws ( https://en.wikipedia.org/wiki/De_Morgan%27s_laws ), because this:

Code: Select all

if (actor.IsA('Pawn') && (Pawn(actor).PlayerReplicationInfo != None) && Pawn(actor).PlayerReplicationInfo.bIsSpectator && Pawn(actor).PlayerReplicationInfo.bWaitingPlayer) {
    //If the actor is a spectating pawn do nothing.
} else {
    ...
}

could as easily be replaced by this:

Code: Select all

if (!actor.IsA('Pawn') || Pawn(actor).PlayerReplicationInfo == None || !Pawn(actor).PlayerReplicationInfo.bIsSpectator || !Pawn(actor).PlayerReplicationInfo.bWaitingPlayer) {
    ...
}

or even if you don't know them:

Code: Select all

isSpectator = actor.IsA('Pawn') && (Pawn(actor).PlayerReplicationInfo != None) && Pawn(actor).PlayerReplicationInfo.bIsSpectator && Pawn(actor).PlayerReplicationInfo.bWaitingPlayer;
if (!isSpectator) {
    ...
}

Yes thanks, I don't know why I left it like that.
(I'll change it when this problem is fixed .o.)

Also, although UScript is case-insensitive, it was a good idea from him to call it "actor" with lowercase rather than "Actor", to make the reference even more plain clear.

Well, it's about good programmer rules lol, it brings to less confusion, so more readability.

Also for debugging I'd simplify that code as much as possible: use constants instead of variables and function calls, leave out unneeded code, add more log messages, don't put it into a timer but trigger that.
If you are able to melt that down to a dozen lines, you could make and publish a test map. Compiling and running that code it via brain may produce headache and dizziness.

True, I was going to do it right today, this is the first simplified version:

Code: Select all

event Timer() {
	local Actor actor;
	local LocationInfo nearestlocInfo, locInfo, newLocInfo;
	
	foreach AllActors(class'Actor', actor)
		if (trackActor(actor) || actor.IsA('Bot') || actor.IsA('PlayerPawn')) {
			//FIXME
			Level.Game.BroadcastMessage(0@actor);
			if (actor.IsA('Pawn') && (Pawn(actor).PlayerReplicationInfo != None) && Pawn(actor).PlayerReplicationInfo.bIsSpectator && Pawn(actor).PlayerReplicationInfo.bWaitingPlayer) {
				//If the actor is a spectating pawn do nothing.
			} else {
				foreach actor.RadiusActors(class'LocationInfo', locInfo, 100) {
					//FIXME
					Level.Game.BroadcastMessage(1@actor);
					nearestlocInfo = locInfo;
				}
				
				//FIXME
				Level.Game.BroadcastMessage(2@actor@nearestlocInfo);
				
				if (nearestlocInfo == None)
					newLocInfo = Spawn(class'LocationInfo', Self,, actor.Location);
			}
		}
}

The problem is still there, player makes it print 0,1,2 while Nali makes it print 0,2(and nearestLocInfo is not None).
if I change the Radius from 100 to 5000, even the Nali goes 0,1,2.
(Also, there's no need to let the Nali moves, the nearestLocInfo should still spawn even if the Nali is next to me, so I'm not hurting him anymore)

EDIT: Looks like Level.Game.BroadcastMessage() wasn't fast enough to print all the logs... I added some logs info, now the code looks like this:

Code: Select all

event Timer() {
	local Actor actor;
	local LocationInfo nearestlocInfo, locInfo, newLocInfo;
	
	//FIXME
	Level.Game.BroadcastMessage(-1@nearestlocInfo);
	
	foreach AllActors(class'Actor', actor)
		if (trackActor(actor) || actor.IsA('Bot') || actor.IsA('PlayerPawn')) {
			//FIXME
			Level.Game.BroadcastMessage(0@actor@nearestlocInfo);
			if (actor.IsA('Pawn') && (Pawn(actor).PlayerReplicationInfo != None) && Pawn(actor).PlayerReplicationInfo.bIsSpectator && Pawn(actor).PlayerReplicationInfo.bWaitingPlayer) {
				//If the actor is a spectating pawn do nothing.
			} else {
				foreach actor.RadiusActors(class'LocationInfo', locInfo, 100) {
					//FIXME
					Level.Game.BroadcastMessage(1@actor@nearestlocInfo);
					nearestlocInfo = locInfo;
					//FIXME
					Level.Game.BroadcastMessage(2@actor@nearestlocInfo);
				}
				
				//FIXME
				Level.Game.BroadcastMessage(3@actor@nearestlocInfo);
				
				if (nearestlocInfo == None) {
					newLocInfo = Spawn(class'LocationInfo', Self,, actor.Location);
					//FIXME
					Level.Game.BroadcastMessage(4@actor@nearestlocInfo@newLocInfo);
				}
			}
		}
}

Logs I get with BroadcastMessage():

Code: Select all

ScriptLog: -1 None
ScriptLog: 0 CTF-Niven.TMale2 None
ScriptLog: 1 CTF-Niven.TMale2 None
ScriptLog: 2 CTF-Niven.TMale2 CTF-Niven.LocationInfo0
ScriptLog: 3 CTF-Niven.TMale2 CTF-Niven.LocationInfo0
ScriptLog: 0 CTF-Niven.NaliPriest0 CTF-Niven.LocationInfo0
ScriptLog: 1 CTF-Niven.NaliPriest0 CTF-Niven.LocationInfo0

Logs I get with Log():

Code: Select all

ScriptLog: -1 None
ScriptLog: 0 CTF-Niven.TMale2 None
ScriptLog: 1 CTF-Niven.TMale2 None
ScriptLog: 2 CTF-Niven.TMale2 CTF-Niven.LocationInfo0
ScriptLog: 3 CTF-Niven.TMale2 CTF-Niven.LocationInfo0
ScriptLog: 0 CTF-Niven.NaliPriest0 CTF-Niven.LocationInfo0
ScriptLog: 1 CTF-Niven.NaliPriest0 CTF-Niven.LocationInfo0
ScriptLog: 2 CTF-Niven.NaliPriest0 CTF-Niven.LocationInfo0
ScriptLog: 3 CTF-Niven.NaliPriest0 CTF-Niven.LocationInfo0

This has a lot more sense now -__-".

It is very annoying to debug it through logs instead than broadcasted messages when you're working with visible stuff in game

EDIT 2: WAIITTTT, I changed the code back to this:

Code: Select all

event Timer() {
	local Actor actor;
	local LocationInfo nearestlocInfo, locInfo, newLocInfo;
	
	foreach AllActors(class'Actor', actor)
		if (trackActor(actor) || actor.IsA('Bot') || actor.IsA('PlayerPawn')) {
			//FIXME
			Log(0@actor);
			if (actor.IsA('Pawn') && (Pawn(actor).PlayerReplicationInfo != None) && Pawn(actor).PlayerReplicationInfo.bIsSpectator && Pawn(actor).PlayerReplicationInfo.bWaitingPlayer) {
				//If the actor is a spectating pawn do nothing.
			} else {
				foreach actor.RadiusActors(class'LocationInfo', locInfo, minDist - actor.CollisionRadius, actor.Location) {
					//FIXME
					Log(1@actor);
					if (locInfo.isGoodCandidate(actor.class))
						if ((nearestlocInfo == None) || VSize(locInfo.Location - actor.Location) < VSize(nearestlocInfo.Location - actor.Location))
							if (FastTrace(actor.Location, locInfo.Location))
								nearestlocInfo = locInfo;
				}
				
				//FIXME - nearestLocInfo is never None for the Nali even if the logs show 0 and 3 only.
				//(case steps:
				//1. You start the match alone;
				//2. You don't move and spawn a Nali;
				//3. You hurt the Nali and let it moves away.)
				Log(2@actor@nearestlocInfo);
				
				if (nearestlocInfo == None) {
					newLocInfo = Spawn(class'LocationInfo', Self,, actor.Location);
					newLocInfo.actorClass = actor.class;
					newLocInfo.attentionLevel++;
				} else {
					nearestlocInfo.attentionLevel++;
				}
			}
		}
}

Replacing messages with logs, but logs came back like this:

Code: Select all

ScriptLog: 0 CTF-Niven.TMale2
ScriptLog: 1 CTF-Niven.TMale2
ScriptLog: 2 Botpack.TMale1 Botpack.TMale1 Vero
ScriptLog: 3 CTF-Niven.TMale2 CTF-Niven.LocationInfo2
ScriptLog: 0 CTF-Niven.NaliPriest0
ScriptLog: 3 CTF-Niven.NaliPriest0 CTF-Niven.LocationInfo2

F*** that o\/o

Maybe code flow is too fast even for native Logs?
(The timer interval is set to 1)

EDIT 3: The issue only occurs when the Nali moves away from me, if he stays next to me, the logs for the Nali are 0,1,2, but if he moves away the logs for the Nali print 0,2. In both the cases, when it prints 2, the nearestLocInfo is still the same, the one that is used by the player(which is a bug since a new one should be spawned right for the Nali class instead).

[Feralidragon]

[...] and since you're not resetting the nearestLocInfo reference to None at the end of each iteration, the next iteration will still have it set to whichever reference it was set previously. [...]

EDIT: In other words:

Code: Select all

if (nearestlocInfo == None) {
    newLocInfo = Spawn(class'LocationInfo', Self,, actor.Location);
    newLocInfo.actorClass = actor.class;
    newLocInfo.attentionLevel++;
} else {
    nearestlocInfo.attentionLevel++;
    nearestlocInfo = None; // <-- add this
}

[PrinceOfFunky]

[...] and since you're not resetting the nearestLocInfo reference to None at the end of each iteration, the next iteration will still have it set to whichever reference it was set previously. [...]

EDIT: In other words:

Code: Select all

if (nearestlocInfo == None) {
    newLocInfo = Spawn(class'LocationInfo', Self,, actor.Location);
    newLocInfo.actorClass = actor.class;
    newLocInfo.attentionLevel++;
} else {
    nearestlocInfo.attentionLevel++;
    nearestlocInfo = None; // <-- add this
}

OH GAWD O.O I thought you were referring to the foreach actor.RadiusActors(), but that's true there's the other foreach! I'm becoming old
Thanks, I'll try it now!

EDIT: Everything works fine now Thanks everyone! I wonder if there was a point when I removed that None assignment(since it worked fine some months ago).

[Feralidragon]

I wonder if there was a point when I removed that None assignment(since it worked fine some months ago).

This is why you should use version control, like Git.