User Tag List

Page 1 of 2 12 LastLast
Results 1 to 10 of 17
  1. #1
    Whicked Sick Higor's Avatar
    Join Date
    Apr 2012
    Location
    Full sail ahead!
    Posts
    3,676
    Country:

    Code review: sgProtector

    Looking into sgProtector and sgSProtector's enemy finding methods.


    ShouldAttack:
    Code:
    function bool ShouldAttack(Pawn enemy)
    {
        local UT_Invisibility inv;
        local bool bInv;
    	local Inventory RubberSuit;
    	
    	RubberSuit = enemy.FindInventoryType(class'WildcardsRubberSuit');
    
        if ( enemy == None )
            return false;
    
        if ( !FastTrace(enemy.Location) )
            return false;
    		
        if ( ScriptedPawn(enemy) != None && enemy.Health > 10 )
            return true;
    		
        if ( sgBuilding(enemy) != None )
        {
            if ( sgBuilding(enemy).Team == Team || sgBuilding(enemy).Energy < 0 )
                return false;
        }
        else if ( enemy.PlayerReplicationInfo == None ||
          enemy.PlayerReplicationInfo.Team == Team ||
          enemy.Health <= 10 || RubberSuit != none ||
          !enemy.bProjTarget)
            return false;
        return true;
    }
    This function is called within a RadiusActors iterator, so we don't need to check for (Enemy == none).
    Second, we shouldn't try finding the rubbersuit until all the other checks have failed.
    Third, we should use FastTrace as final positive check, so we use it the least amount of times.
    We'll also remove some unused variables.

    Suggested code:
    Code:
    function bool ShouldAttack(Pawn enemy)
    {
        if ( ScriptedPawn(enemy) != None )
        {
            if ( enemy.Health < 10 )
                return false;
            return FastTrace( enemy.Location);
        }
    		
        if ( sgBuilding(enemy) != None )
        {
            if ( sgBuilding(enemy).Team == Team || sgBuilding(enemy).Energy < 0 )
                return false;
        }
        else if ( enemy.PlayerReplicationInfo == None ||
          enemy.PlayerReplicationInfo.Team == Team ||
          enemy.Health <= 10 || !enemy.bProjTarget || (enemy.FindInventoryType(class'WildcardsRubberSuit') != none) )
            return false;
        return FastTrace(enemy.Location);
    }
    No new variablies initialized, RubberSuit is found right if when need to find it and the trace checks are done as final condition.

    EDIT 1:
    FindEnemy:
    Code:
    function Pawn FindEnemy()
    {
        local Pawn p, target;
    
        SightRadius = 1000 + 1750*Grade;
    
        foreach RadiusActors(class'Pawn', p, SightRadius)
            if ( ShouldAttack(p) && (target == None ||
              VSize(p.Location - Location) < VSize(target.Location - Location)) )
                target = p;
    
        return target;
    }
    ShouldAttack requires more resources than the distance checks, we should invert these checks.
    We shouldn't search further away from current enemy if we have one.

    Suggested code:
    Code:
    function Pawn FindEnemy()
    {
    	local Pawn p;
    	local float eDist;
    
    	//Should be applied during upgrade, does the function Upgraded() properly work?
    //	SightRadius = 1000 + 1750*Grade;
    
    	if ( Enemy != none )
    	{
    		eDist = VSize( Enemy.Location - Location);
    		if ( (eDist > SightRadius) || !ShouldAttack(Enemy) )
    			Enemy = none;
    		else
    		{
    			eDist -= 1;
    			foreach RadiusActors(class'Pawn', p, eDist)
    				if ( (VSize(p.Location - Location) < VSize(Enemy.Location - Location)) && ShouldAttack(p) )
    					Enemy = p;
    			return Enemy;
    		}
    	}
    
    	foreach RadiusActors(class'Pawn', p, SightRadius)
    		if ( (Enemy == None || VSize(p.Location - Location) < VSize(Enemy.Location - Location)) && ShouldAttack(p) )
    			Enemy = p;
    
        return Enemy;
    }
    We store Enemy for future uses, if enemy is still valid, we try finding one at a closer distance.
    Expensive checks placed last as well.

    Other changes:
    Add:
    Code:
    simulated function Upgraded()
    {
    	SightRadius = 1000 + 1750*Grade;
    }
    And in the defaultproperties block add:
    Code:
         SightRadius = 1000;
    Checking on sgSProtector:
    Code:
    function bool ShouldAttack(Pawn enemy)
    {
        return Super.ShouldAttack(enemy);
    }
    sgSProtector is subclass of sgProtector, you should remove this.
    Last edited by Higor; 05-24-2012 at 02:04 PM.

  2. #2
    Administrator SAM's Avatar
    Join Date
    Jan 2011
    Posts
    8,296
    Country:
    I believe we lost some posts on this subject due to the erratic downtime which has now been resolved. Please feel free to discuss!

  3. #3
    Rampage Feralidragon's Avatar
    Join Date
    Jan 2011
    Posts
    374
    Country:
    The only thing I think it's missing in that, is not about optimization, is simply about flexibility:
    Code:
    enemy.FindInventoryType(class'WildcardsRubberSuit')
    Could be:
    Code:
    enemy.FindInventoryType(RubberSuitClass)
    where the RubberSuitClass would be a:
    Code:
    var() Class<Inventory> RubberSuitClass;
    Just to avoid it to be hardcoded, which isn't very good (although admittedly I did it and still do it myself sometimes).
    As for the rest, totally agree with the optimization.

  4. #4
    Whicked Sick Higor's Avatar
    Join Date
    Apr 2012
    Location
    Full sail ahead!
    Posts
    3,676
    Country:
    Another discussion point here:

    From what can be seen in the Protector's behaviour is that they always pick the nearest enemy.
    What if this nearest enemy is stored in a variable (enemy?), and for the following enemy checks, only check the distance to that enemy - 1.0 unit? Of course, never forgetting of checking if current enemy is still eligible.

    This way if the sgProtector finds an enemy at distance 500, all following enemy-finding iterations must be performed with a radius of 499 units, after all, if no new enemy is found we keep the old enemy.

    The rubbersuit check was already a hardcoded class, but indeed it would be good to have a DontShootIfCarrying variable for that inventory class.

  5. #5
    Rampage Feralidragon's Avatar
    Join Date
    Jan 2011
    Posts
    374
    Country:
    Quote Originally Posted by Higor View Post

    From what can be seen in the Protector's behaviour is that they always pick the nearest enemy.
    What if this nearest enemy is stored in a variable (enemy?), and for the following enemy checks, only check the distance to that enemy - 1.0 unit? Of course, never forgetting of checking if current enemy is still eligible.

    This way if the sgProtector finds an enemy at distance 500, all following enemy-finding iterations must be performed with a radius of 499 units, after all, if no new enemy is found we keep the old enemy.
    That's a very good idea, just don't forget 2 things:
    1 - you need to make an extra fast trace to the current enemy before you do a smaller radius check, since he can go out of sight at any given moment;
    2 - you need to check if that distance is within the max protector current range (since they vary with level).

    Also, another very minor thing, but still quite important nonetheless, although I don't see any direct problems with that solution, don't forget that UT works with floating point variables, which means lack of precision. On a direct floating point context, 1unit is precise enough, but in a radial perspective (Pitagoras Theorem) the calculations become less precise and precision will be lost, so that 1 unit of difference may not be 1 exact unit at some checks and may still detect the enemy as well, or not, depending on the final value of the calculation at c++ level (which I believe uses floating point as well?).

  6. #6
    Whicked Sick Higor's Avatar
    Join Date
    Apr 2012
    Location
    Full sail ahead!
    Posts
    3,676
    Country:
    Edited first post, added another function rewrite.

    PD: In case we indeed have a float precision error, the distance check should prevent ShouldAttack() being called twice on it, ugly but safe.
    PD2: I notice sgBuilding has an Upgraded() method, SightRadius could be set to a default of 1000, and modified on each Upgraded() call (only needed serverside).

    Check again, now Upgraded() sets sight radius.
    Last edited by Higor; 05-24-2012 at 02:05 PM.

  7. #7
    Moderator .seVered.]['s Avatar
    Join Date
    Jun 2011
    Location
    Near a River and Under a Bridge
    Posts
    2,125
    Country:
    Code:
    function Shoot(Pawn target){
        local vector    fireSpot,
                        projStart;
        local Projectile
                        proj;
        local rotator   shootDirection;
    
    
        if ( target == None || VSize(target.Location - Location) > SightRadius )
            return;
    
    
        shootDirection = rotator(target.Location - Location);
    
    
        PlaySound(FireSound, SLOT_None, 0.75);
    
    
        if ( Grade > 2 )
        {
            fireSpot = target.Location + FMin(1, 1.1 + 0.6 * FRand()) *
              target.Velocity * VSize(target.Location - Location) /
              ProjectileType.default.Speed;
    
    
            if ( !FastTrace(fireSpot, ProjStart) )
                fireSpot = 0.5 * (fireSpot + target.Location);
    
    
            shootDirection = Rotator(fireSpot - Location);
        }
    
    
        Spawn(ProjectileType, Self,, Location, shootDirection);
    }
    I Love how big the CodeBox is BTW (< thank you $carface)

    Pythagorean theorem for calculating the point in 3d space is only half of it. Finding the fireSpot also requires "bearing". Does it base this on the direction you are facing when you build it, like the teleport is or is it the same for all SP's?


    It appears that the SP is trying to anticipate where the target is "going" to be (fireSpot=). Even at level 5 the ProjectileType.default.Speed is not fast enough, as I can outrun even 2 SP's close together. (unless player velocity were actually adjusted according to health status ( less health, less speed) which it is not. So increase the ProjectileType.default.Speed or adjust the forumla.

    The fast trace is done in the Shoot(), why is it done again in the ShouldAttack()?

    The unused variables could have been used to decrease the accuracy of the Spawn(ProjectileType, Self,, Location, shootDirection) if the target had invisibility suit on, but was removed prior; and the bInv; variable might have also part of that. Perhaps this could be RE-inserted?


    Why remove the subclass of sgSProtector?.

  8. #8
    Whicked Sick Higor's Avatar
    Join Date
    Apr 2012
    Location
    Full sail ahead!
    Posts
    3,676
    Country:
    It would appear that the FastTrace check was a sanity check left by the previous coder, we should see under what contexts is Shoot called, if under every circumstance this function is called, the target was already checked by a FastTrace, we can remove it from here.
    Anticipation has a randomized effect here, I think it was intended, so I won't touch it.

    EDIT: We're not removing sgSProtector, only a function that's not necessary.

  9. #9
    Moderator .seVered.]['s Avatar
    Join Date
    Jun 2011
    Location
    Near a River and Under a Bridge
    Posts
    2,125
    Country:

    Wiki.SiegeUltimate.com

    Can we move these and other posts to their own Department, sort of create a wiki.Siege Thread collection? These posts could prove quite useful to future targets. It would be nice to have them ALL in one place.

  10. #10
    The Best There Ever Will Be! |uK|B|aZe//.'s Avatar
    Join Date
    Jan 2011
    Location
    London, United Kingdom
    Posts
    6,860
    i personally dont wanna read all this

    but as long as we can trust you and you have the intentions of doing good i dont see why we cannot use your help

    also please just make sure siege runs optimally and not like a huge turd as it is now

    im sure you will do a good job! thanks

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Similar Threads

  1. Code review: sgHUD
    By Higor in forum #siegepug Discussion
    Replies: 6
    Last Post: 06-01-2012, 02:45 PM
  2. Code review: sgTeleporter
    By Higor in forum #siegepug Discussion
    Replies: 12
    Last Post: 05-26-2012, 08:24 PM
  3. Code review: Mines 2
    By Higor in forum #siegepug Discussion
    Replies: 5
    Last Post: 05-26-2012, 02:49 PM
  4. Code review: sgProtector
    By Higor in forum #siegepug Discussion
    Replies: 9
    Last Post: 05-24-2012, 03:21 PM
  5. Code review: Mines
    By Higor in forum #siegepug Discussion
    Replies: 20
    Last Post: 05-21-2012, 09:36 PM

Tags for this Thread

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •