User Tag List

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

    Code review: Supplier ammo-giving optimization.

    This one reduces the processing time of the suppliers when giving ammo and upgrading armor.
    It still does not regenerate thrown weapons, that can be added at the expense of losing speed.

    First thing first, we need an extra variable in the class sgSupplier:
    Code:
    var int iWeaponClasses;
    This adds a lot of flexibility in code by reducing iteration count if a weapon class fails to load (mods?).


    Now, when loading the weapon classes, we add a some new code after the function:
    Code:
    static function LoadWeaponClasses()
    {
        local int i;
        local class<Weapon> weaponClass;
    
        default.ClassesLoaded = true;
        for ( i = 0; i < 9; i++ )
            if ( default.Weapons[i] != "" )
                default.WeaponClasses[i] =
                  class<Weapon>(DynamicLoadObject(default.Weapons[i],
                  class'Class'));
    
    	//Keep array compacted, faster if weapon fails to load
    	default.iWeaponClasses = 9;
    	for ( i=0 ; i<default.iWeaponClasses ; i++ )
    		if ( default.WeaponClasses[i] == none )
    		{
    			default.WeaponClasses[i] = default.WeaponClasses[--default.iWeaponClasses];
    			default.WeaponClasses[default.iWeaponClasses] = none;
    			--i; //Loop this element again
    		}
    }
    That's part of the process of keeping the weapon class table compacted and with a ceiling index (weapon class count), it sorta makes it work like a dynamic array.


    Now we go to the main Supply function:
    Code:
    function Supply(Pawn target)
    {
    
        local int numWeapons, i, j;
        local Inventory inv;
    	local sgArmor theArmor;
    
    	if ( bDisabledByEMP )
    		return;
    
        if ( !default.ClassesLoaded )
            LoadWeaponClasses();
    
        numWeapons = min(GetWeaponCount(), default.iWeaponClasses);
    	for ( inv=target.Inventory ; inv!=none ; inv=inv.Inventory )
    	{
    		if ( (theArmor == none) && (sgArmor(Inv) != none) )
    		{
    			theArmor = sgArmor(Inv);
    			continue;
    		}
    
    		if ( (Weapon(Inv) == none) || (Weapon(Inv).AmmoType == none) )
    			continue;
    
    		for ( i=0 ; i<numWeapons ; i++ )
    		{
    			if ( Inv.class == default.WeaponClasses[i] )
    			{
    				j++;
    				if ( Weapon(inv).AmmoType.AmmoAmount < Weapon(inv).PickupAmmoCount / 2 )
    	                Weapon(inv).AmmoType.AmmoAmount = Weapon(inv).PickupAmmoCount / 2;
    	            else if ( FRand() < float(Weapon(inv).AmmoType.MaxAmmo) / 400 )
    					Weapon(inv).AmmoType.AmmoAmount = FMin(
    						Weapon(inv).AmmoType.AmmoAmount + 1 +
    						int(FRand()*Grade/2*EquipRate),
    						Weapon(inv).AmmoType.MaxAmmo);
    				if ( j >= numWeapons )
    					Goto WEAPONS_READY;
    				break;
    			}
    		}
    	}
    
    	WEAPONS_READY:
    	if ( theArmor != none )
    		Goto UPGRADE_ARMOR;
    	
    	while ( inv != none )
    	{
    		if ( sgArmor(Inv) != none )
    		{
    			theArmor = sgArmor(Inv);
    			Goto UPGRADE_ARMOR;
    		}
    		inv = inv.Inventory;
    	}
    
    	theArmor = Spawn( class'sgArmor');
    	if ( theArmor == none )
    		Goto PLAY_SOUND;
    	theArmor.GiveTo( Target);
    
    	UPGRADE_ARMOR:
    	if ( FRand() < 0.1 + (Grade/15) && inv.Charge < 25 + Grade*25 )
    		++theArmor.Charge;
    
    
    	PLAY_SOUND:
        if ( FRand() < 0.2 )
            target.PlaySound(sound'sgMedia.sgStockUp', SLOT_Misc,
              target.SoundDampening*2.5);
    }
    Code here can result illegible to some, it contains jumps to ensure maximum speed, it's also very compacted and adding an extra item to upgrade would require a reformulation of some loops.

    Explanation step by step:
    - Instead of looping the entire Inventory chain for each weapon/armor, we simply go through the Inventory chain once and start making our weapon checks there.
    If we happen to catch the armor we also store it...
    Now we check in the first loop if it's a valid suppliable weapon, if it is, we start checking among our weapon table (limited by Supplier level), if weapon is in the table, we add ammo and count J up.
    If the J counter reaches the maximum weapons to supply (depends on Supplier level), we immediately leave the main loop and the subloop.
    Notice that when we leave both loops, INV variable doesn't change or anything, we're still in the middle of the inventory chain...

    - We left the loop, either by having it finished (entire inventory chain) or by jumping out into WEAPONS_READY, let's check if we got the armor during these iterations...
    We got it!, skip some code by jumping to UPGRADE_ARMOR.
    We didn't get it... remember INV? We continue looping through the Inventor chain in a much simpler loop meant to find this armor.
    If we found the armor, we instantly jump to UPGRADE_ARMOR.
    If we didn't, we create the armor, if armor fails to be created, we jump to PLAY_SOUND.

    - We reach UPGRADE_ARMOR, either by finding it or by making it.
    The code difference here? There's already a maximum armor check on the conditional, so using the ++ preoperator to add one is entirely the same and much faster than the previous method.

    - Code keeps going, we now reach PLAY_SOUND.
    This is the same as before, PLAY_SOUND is added here to skip armor upgrade if we fail to create it.


    =======
    Super supplier has identical code for both functions, replace them as well, add the extra variable and you're set.


    Trivia:
    Did you know this works in unrealscript?
    Code:
    	if ( aNumber == 6 )
    		Goto DO_SOMETHING;
    
    	if ( false ) //Never enter this code chunk by normal means
    	{
    		DO_SOMETHING:
    		TestFunction();
    	}
    or

    Code:
    	if ( aNumber == 6 )
    		Goto AFTER_FUNCTION;
    
    	return;
    
    	AFTER_FUNCTION:
    	TestFunction();
    }

  2. #2
    Moderator .seVered.]['s Avatar
    Join Date
    Jun 2011
    Location
    Near a River and Under a Bridge
    Posts
    2,122
    Country:
    Quote Originally Posted by Higor View Post
    Code:
        if ( aNumber == 6 )
            Goto DO_SOMETHING;
    
        if ( false ) //Never enter this code chunk by normal means
        {
            DO_SOMETHING:
            TestFunction();
        }
    or

    Code:
        if ( aNumber == 6 )
            Goto AFTER_FUNCTION;
    
        return;
    
        AFTER_FUNCTION:
        TestFunction();
    }
    Damn, that ALMOST looks like visual basic... I should just use my VB editor for uScript. What about intrinsic subroutine's? (GOSUB). What other c++ routine's can you apply to the NukeLauncher routine's, perhaps a DODGE while your flying the nuke... :encouragement:

    I already saw your post about the the nested dynamic array ...nice and clean, except your comment is not INDENTED :emmersed: (*wink)

    Code:
    static function LoadWeaponClasses()
    {
        local int i;
        local class<Weapon> weaponClass;
    
        default.ClassesLoaded = true;
        for ( i = 0; i < 9; i++ )
            if ( default.Weapons[i] != "" )
                default.WeaponClasses[i] =
                  class<Weapon>(DynamicLoadObject(default.Weapons[i],
                  class'Class'));
    
    -> //Keep array compacted, faster if weapon fails to load  <--
        default.iWeaponClasses = 9;
        for ( i=0 ; i<default.iWeaponClasses ; i++ )
            if ( default.WeaponClasses[i] == none )
            {
                default.WeaponClasses[i] = default.WeaponClasses[--default.iWeaponClasses];
                default.WeaponClasses[default.iWeaponClasses] = none;
                --i; //Loop this element again
            }
    }
    Your a lawyer right... seem's like that should have been your FALLBACK career, instead of your primary. Your quite good at this ... stuff.

  3. #3
    Rampage Feralidragon's Avatar
    Join Date
    Jan 2011
    Posts
    374
    Country:
    Oh noes.... I asked this in the other topic, but looking at this one it seems you're actually serious in using goto's...

    Let me explain the following: goto's are BAD. Period. Don't use them, don't build algorithms with them, as you stated it makes the code confusing (unnecessarily I must add), which leads inevitably to VERY UNSTABLE code (you can argue back saying your code "works" with these goto's, but it's not correct anyway as the code looses structure and readability later, which is what enhances our human nature of making mistakes while building something, thus by loosing structure you loose stability). You can build exactly the same algorithms in a much more simpler way and have the same speed (sometimes a bit slower, others a bit faster, but always along the same) using proper algorithmic ways using simple "if", "switch", "while", "for", etc.

    In uscript in particular, goto should only be used in state code for "latent code" only (since "while" and "for" do not support at all latent instructions within them in uscript), and that's the only place you see goto's used, and even there their usage is as brief as possible.

    So, if you're going to seriously reviewing and show SW and others new algorithms to optimize things up in Siege and potentially other things, PLEASE don't use nor promote the usage of goto's (if you want to keep using them yourself, fine, but don't make it an example to follow), that's just a bad algorithmic example (and this isn't just a matter of opinion, just put "why goto is bad" in google and you have tons of places explaining why).
    Last edited by Feralidragon; 05-12-2012 at 02:30 PM.

  4. #4
    Moderator .seVered.]['s Avatar
    Join Date
    Jun 2011
    Location
    Near a River and Under a Bridge
    Posts
    2,122
    Country:
    That's what I said, what about the intrinsic subroutine's?

    Quote Originally Posted by .se\/ered.][ View Post
    Damn, that ALMOST looks like visual basic... I should just use my VB editor for uScript. What about intrinsic subroutine's? (GOSUB). What other c++ routine's can you apply to the NukeLauncher routine's, perhaps a DODGE while your flying the nuke... :encouragement:
    When you "jump" out of the ForNext loop using a gosub (aka Function), the stack pointers are maintained and a new stack does not need to be initialized, reducing overhead and maintaining stability (there are memory restrictions and overhead costs to stacks, aren't there?).

  5. #5
    Whicked Sick Higor's Avatar
    Join Date
    Apr 2012
    Location
    Full sail ahead!
    Posts
    3,675
    Country:
    Readability depends on what the programmer feels more confortable with after all, I use both gotos and For, While loops.
    Since optimization was the goal, I simply chose gotos, if SilverWing or any other Siege developer doesn't want these gotos (maybe they want to implement more into the supplier), I'll reformulate the functions, excepting for that first goto that leads out of a nested loop.

  6. #6
    Rampage Feralidragon's Avatar
    Join Date
    Jan 2011
    Posts
    374
    Country:
    Quote Originally Posted by Higor View Post
    Readability depends on what the programmer feels more confortable with after all, I use both gotos and For, While loops.
    Nope, you're mistaken on what "readability" is.
    Readability is defined from the standard and generic point of view, when you say something is readable, then it means anyone should be able to read it (in case of programming, anyone with minimal knowledge should be able to read it perfectly fine).
    It doesn't matter what you're more comfortable with personally, that doesn't necessarily make it "readable" at all.
    Example:
    Code:
    _ = (0x0012 + \027 + (~15 + 1)) << 1;
    What just did I do there? Is it "readable"? Let's say that's how I code (obviously not, but just for exemplification).
    For me is perfectly readable because I built it with everything in my mind and since I was comfortable with it, for you, perhaps you're not used to that kind of notation but you will still understand it nonetheless, but it will make you think slightly rather than interpret it right away, to others they may or not even figure out what I just put there and why when there are more simple and correct approaches.

    In resume (for the ones that don't now what I did there): "_" is an integer var, but let's substitute it for "_var" for readability:
    Code:
    _var = (3 + 23 - 15) * 2;
    Now it's "readable".


    Quote Originally Posted by Higor View Post
    Since optimization was the goal, I simply chose gotos, if SilverWing or any other Siege developer doesn't want these gotos (maybe they want to implement more into the supplier), I'll reformulate the functions, excepting for that first goto that leads out of a nested loop.
    You're keeping the "goto"...? You know, you can achieve exactly the same objective without goto's without loosing performance, "while" and "for" are your best friends for it and they exist for the sole purpose of not having to use goto's for cycles and such (that's why there's even a "do while"), plus breaking out from a "for" loop without being with a "return" is not good, that's what "while" is for (although I must admit I slipped in that mistake myself in some rare occasions).

    Anyway, goto's aren't the holy grail of code optimization, using goto's as "max performance" excuse, well, that's really nothing but an excuse to not write what should be good and clean code, they're bad as they can be overused and make the code unreadable, buggy and unnecessarily complex (and you've already shown you abuse them quite a bit, 4 goto's in a single function....), and they're often a sign of bad written algorithms.
    If you reformulate the code without goto's, you will see that you will make a better and cleaner algorithm, which is actually the correct thing to do, but if you want still to use them and shove it down SW if he accepts it as it is, well, sorry but you're serving a very bad coding example for a potential future developer (or developers) and I think you should seriously rethink what you're posting up.
    Personally, I'm already happy you will take out most of them, but you should really take *all* of them out.

    From there, if you use them personally, no one has anything to do with it but you, and I have a few bad habits myself as well which I am seriously working out and trying to correct them (and the ones I corrected so far made me a much better programmer than I was months or years ago, "good practices" sure make a lot of difference), but when I teach or show up something for people to understand, I make always sure I show the correct thing to do, and not necessarily how I do it personally.
    Last edited by Feralidragon; 05-12-2012 at 08:19 PM.

  7. #7
    Whicked Sick terminator's Avatar
    Join Date
    Jan 2011
    Location
    Croatia, Pula
    Posts
    1,281
    Country:
    maybe outta thread but please if u know how to, fix PLATFORMS! Thanks )

  8. #8
    Whicked Sick |uK|Grimreaper's Avatar
    Join Date
    Feb 2011
    Posts
    1,316
    Country:
    wtf since when does higor know coding XD

    AND FIX THE PLATFORM BUG AFTER IT GOT REMOVED


  9. #9
    Whicked Sick Higor's Avatar
    Join Date
    Apr 2012
    Location
    Full sail ahead!
    Posts
    3,675
    Country:
    I haven't been around for much to know what happens with platforms... mind telling me what's wrong with it?

  10. #10
    Whicked Sick |uK|Grimreaper's Avatar
    Join Date
    Feb 2011
    Posts
    1,316
    Country:
    when a platform is build and it gets removed the spot where the platform was is buggy you lag trough it try it out on the server


Thread Information

Users Browsing this Thread

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

Similar Threads

  1. Code review: Supplier ammo-giving optimization.
    By Higor in forum #siegepug Discussion
    Replies: 19
    Last Post: 06-07-2012, 08:34 AM
  2. Code review: sgHUD
    By Higor in forum Code Reviews
    Replies: 6
    Last Post: 06-01-2012, 03:45 PM
  3. Replies: 4
    Last Post: 05-13-2012, 12:41 AM
  4. Code review: Supplier (50%) and WeaponFiding optimization.
    By Higor in forum #siegepug Discussion
    Replies: 4
    Last Post: 05-13-2012, 12:41 AM
  5. Supplier Ammo Increase
    By SilverWing in forum zp| * * * -=[SIEGE]=- |uK| One Night Stand -=[SIEGE]=- Server * * *
    Replies: 9
    Last Post: 02-27-2012, 11:11 AM

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
  •