New Map: Blackout-b1 first playable version

[SlackerLinux]

Nobody's going to seriously consider including the AMP code in trunk unless the style is cleaned up. That means whitespace and brackets need to be consistent with the rest of the codebase and each other. It also means that the code looks like a part of Tremulous, not something that was hacked on an differentiated like some separate thing, so no ==== TROY AMP STUFF ==== everywhere; hell no AMP name at all. Also no huge blocks of code that's been commented out. String copy functions need to use the ones the rest of the codebase uses, not define their own for no reason. And that's just what I see in scrolling through it once. :p

a days work at best

if uniq doesnt want to do it i can prob clean it up a little

[UniqPhoeniX]

I'm a mapper/modeler not a coder devhc ported this version of AMP (not sure if F50 helped ?) if you do plan on cleaning it up, you might want to get latest src from devhc (atm only a couple of changes afaik)

[SlackerLinux]

this is a whitespace and general style cleanup of the 1st patch there is some changes to the code but not much ill change all the string functions next just wanted to get a whitespace clean version out for anyone working on this

AMP GPP patch V1.1
Old patch
AMP GPP patch V1


its weird someone working on this KNEW how to format properly there was code formatted correctly in chunks i have no clue why they didn't fix it all up

EDIT: took about 30mins and that's with changing it all manually i did it that way to get to know the code more and try to spot obvious bugs i did add a few notes here and there

EDIT2: 1.1 removed the debugging functions i didn't really like them also de-ampified some more code full change description should be in the .patch but for the lazy

Code:

[PATCH] AMP gpp port v1.1

Changes since original patch:
  * cleaned up whitespace and code style to match surrounding code
  * changed and simplfied Use_Target_if function a little
  * included devHC's buildpoint fix patch
  * Removed debugAMP cvar and related code it was the only thing using its own string functions
  * renamed AMPintVAR to just intVAR
  * renamed g_AMPstagelock to g_StageLock

[gimhael]

Good Job !

That debug code was bad, but now you should remove that ugly global var g_trigger_success too, it was read only in the debug code.

And of course it should use existing code for class/weapon changes etc. whenever possible 

[UniqPhoeniX]

However ugly the debug code was, I have to point out that blackout would not have happened without any debug.
The debugging used to be very different, probably cleaner in code (when something triggered something else it just printed info about both entities, 1 line before and (if the cvar was set to 2) another line after the triggering), but not quite as useful for me as a mapper. The debug in the current amp version does have its own problems tho, like spamming (lots of) unnecessary info. If the cleaned up version is used, I recommend adding at least some sort of debugging info.

[SlackerLinux]

Good Job !

That debug code was bad, but now you should remove that ugly global var g_trigger_success too, it was read only in the debug code.

And of course it should use existing code for class/weapon changes etc. whenever possible 

g_trigger_success is now gone and ill look into stuff like the weapon/other code soon(im taking it slow i haven't played around with map related stuff before) i also removed some code that was commented out and looked to me to be redundant

debugging of some sort can be added later i just care about getting the code clean enough for consideration also what work has devHC got does he have a public repo somewhere with it

[F50]

I have created a slightly updated patch that removed a goto statement , removed a useless loop, and fixed a bug where target_force_weapon would still call g_UseTarget() if there was not enough room to change to a battlesuit (effectively causing the inventory of the entity to be used up, while not granting the player a battlesuit).

[/dev/humancontroller]

here's my current version of the AMP patchset. the following patches r to be applied to revision 2184 of the gpp branch of the icculus.org tremulous repository, in order.
1. fix systeminfo transmission [we r waiting for the next ioq3 merge to include this fix]
2. fix g_disabled* leak [this fix should have already been included in the repository, but the committers r lagging]
3. AMP for GPP, b4
4. debugging extension for AMP for GPP, b4
alternatively, an all-in-one patch is provided.

[SlackerLinux]

1.1a available with f50's changes ill work in looking at whats different in devhc's build and add those too might take a bit its not as clean as my version(sofar looks like amp-b4 contains some force_weapon/armour cleanup)

latiku7 any chance of looking at 1.1a now that some if its cleaned up and maybe give a list of what you would like to have done to get this ready

oh and 1.1a link
Amp patch 1.1a

[Lakitu7]

At a glance:

Adding commented-out lines

blocks that add brackets around one-line statements in unrelated code

whitespace failures
+  if (!s || !strstr( s, ".md3" ))
+  G_SpawnInt("resetval","254", &ent->ResetValue);
+  if( (total != test) && !(ent->TrigOnlyRise && total) )
+  if( !(self->spawnflags & 2) )

Use_Target_force_win can probably use humanWin/alienWin for less redundancy?

Bad comment
+          // note you can't carry more than one weapon anymore ?

Block in g_utils.c removes a line of whitespace for no reason

You still have G_CopyString but don't even use it anymore

[SlackerLinux]

At a glance:

Adding commented-out lines

blocks that add brackets around one-line statements in unrelated code

whitespace failures
+  if (!s || !strstr( s, ".md3" ))
+  G_SpawnInt("resetval","254", &ent->ResetValue);
+  if( (total != test) && !(ent->TrigOnlyRise && total) )
+  if( !(self->spawnflags & 2) )

Use_Target_force_win can probably use humanWin/alienWin for less redundancy?

Bad comment
+          // note you can't carry more than one weapon anymore ?

Block in g_utils.c removes a line of whitespace for no reason

You still have G_CopyString but don't even use it anymore

G_Copystring is gone now i thought i deleted it with the debug code my bad
rest ill fix up while i add devhc's changes

[/dev/humancontroller]

while i add devhc's changes

i don't see what u r trying to accomplish, other than perhaps prepare a more readable version of the code. i will have to fully rewrite each function (preferring the original as the base) to produce clean and correct code anyway. until every piece of the AMP code gets rewritten, this piece of shit patch is going nowhere near the trunk.

[Lakitu7]

If the logic is actually terrible, then yes, all this refactoring will do is get it to the point where someone will actually read it enough to say so.

[SlackerLinux]

while i add devhc's changes

i don't see what u r trying to accomplish, other than perhaps prepare a more readable version of the code. i will have to fully rewrite each function (preferring the original as the base) to produce clean and correct code anyway. until every piece of the AMP code gets rewritten, this piece of shit patch is going nowhere near the trunk.

well afaik the only things that was holding it back was whitespace and formatting and minor code issues so i was going to try to get a copy clean enough to be considered but if your eventually going to rewrite/refactor everything there prob isnt much point cleaning it up yet

[Lakitu7]

Making it readable is necessary first for someone to take the time to read and evaluate it. If it still sucks after that, then further work will be needed. As far as I am concerned, I do not know whether readability is the only problem; however, others (albeit not official folks) have suggested that it is not.

[Loki]

It must be in 1.2 version.

[Haraldx]

I wish my 1.2 GPP would run now...

[Loki]

And the AMP patch should be imported to Tremulous 

[/dev/humancontroller]

here's my current version of the AMP patchset. the following patches r to be applied to revision 2184 of the gpp branch of the icculus.org tremulous repository, in order.
1. fix systeminfo transmission [we r waiting for the next ioq3 merge to include this fix]
2. fix g_disabled* leak [this fix should have already been included in the repository, but the committers r lagging]
3. AMP for GPP, b4
4. debugging extension for AMP for GPP, b4
alternatively, an all-in-one patch is provided.

i update the following parts of the patchset:
3. AMP for GPP, b6
4. debugging extension for AMP for GPP, b6
and of course, the all-in-one patch, b6.

much of the code has been rewritten: more bugs have been fixed, and the code is a lot cleaner. there r still some issues to be addressed, though.

EDIT: -1 refactoring failure

[Chomps123]

Can you pls give me the game.qvm that has the patch on it.

Thanks in advance.

[/dev/humancontroller]

here's my current version of the AMP patchset. the following patches r to be applied to revision 2184 of the gpp branch of the icculus.org tremulous repository, in order.
1. fix systeminfo transmission [we r waiting for the next ioq3 merge to include this fix]
2. fix g_disabled* leak [this fix should have already been included in the repository, but the committers r lagging]
3. AMP for GPP, b4
4. debugging extension for AMP for GPP, b4
alternatively, an all-in-one patch is provided.

i update the following parts of the patchset:
3. AMP for GPP, b6
4. debugging extension for AMP for GPP, b6
and of course, the all-in-one patch, b6.

i update the following parts of the patchset:
3. AMP for GPP, b7
4. debugging extension for AMP for GPP, b7
and of course, the all-in-one patch, b7.
in b7 i've rewritten almost everything, and the 3rd patch is ready for more serious inspection by others.

as requested, here's a compiled game module in QVM format (based on the gpp branch of r2184 of the icculus.org repository, with r2185 merged).

[F50]

I have been hosting AMP and blackout on the Brindus Mod Rotation, and some people have been complaining that the build menu only allows them to build repeaters, and doesn't allow them to evolve. Binds work normally however, only the menu is screwed up, and only clientside.

[/dev/humancontroller]

I have been hosting AMP and blackout on the Brindus Mod Rotation, and some people have been complaining that the build menu only allows them to build repeaters, and doesn't allow them to evolve. Binds work normally however, only the menu is screwed up, and only clientside.

u should use the following fix:

1. fix systeminfo transmission [we r waiting for the next ioq3 merge to include this fix]

note that this requires a recompilation of the server module to be effective. i actually haven't thought of cases where one might not be able to do that.

so i provide an alternative, game-side hack:
(5.) never empty the g_disabled* cvars [only use if u cannot recompile the server module]
the only perceivable downside to having this applied is that clients might get some redundant warnings. and here's the alternative game module in QVM format, which includes the above hack (based on the gpp branch of r2184 of the icculus.org repository, with r2185 merged).

[F50]

1. fix systeminfo transmission [we r waiting for the next ioq3 merge to include this fix]

Isn't this included in the all-in-one patch?

[F50]

Hrm, accidental double post

[/dev/humancontroller]

1. fix systeminfo transmission [we r waiting for the next ioq3 merge to include this fix]

Isn't this included in the all-in-one patch?

1. fix systeminfo transmission [we r waiting for the next ioq3 merge to include this fix]

note that this requires a recompilation of the server module to be effective. i actually haven't thought of cases where one might not be able to do that.

Its included in the all-in-one patch right? And do you mean the tremded?

i have a feeling that u r repeating urself.
yes, the all-in-one patch includes patches 1 to 4.
yes, i mean tremded (or the tremulous binary, whichever is used for serving).

[Lakitu7]

This isn't ready for more serious inspection yet. You know that this is still nowhere near close to usable in trunk. Firstly, it still has the formatting/naming issues that I already pointed out and Slacker already fixed before you stopped him from working on it and ignored all his work. Take out anything called "AMP". Take out the silly ==TROY== comments. Credits go in commit messages and credits lists, not in source comments.

I do not believe that you believe this is remotely acceptable in trunkable code:

Code:

+    if( weapons[ i ] == WP_NONE && !Q_stricmp( q, "ackit" ) ) // AMP FIXME: remove this compat hax
+      weapons[ i ] = WP_HBUILD;

Code:

+  if( attacker && attacker->client && ( mod == MOD_MACHINEGUN || mod == MOD_PAINSAW ||
+        mod == MOD_SHOTGUN || mod == MOD_LASGUN || mod == MOD_MDRIVER ||
+        mod == MOD_CHAINGUN || mod == MOD_ABUILDER_CLAW || mod == MOD_LEVEL0_BITE ||
+        mod == MOD_LEVEL1_CLAW || mod == MOD_LEVEL2_CLAW || mod == MOD_LEVEL2_ZAP ||
+        mod == MOD_LEVEL3_CLAW || mod == MOD_LEVEL3_POUNCE || mod == MOD_LEVEL4_CLAW ||
+        mod == MOD_LEVEL4_TRAMPLE || mod == MOD_LEVEL4_CRUSH ) )

Code:

+    switch( mod )
+    {
+      case MOD_BLASTER:           weapon = WP_BLASTER;        class = PCL_HUMAN;              break;
+      case MOD_MACHINEGUN:        weapon = WP_MACHINEGUN;     class = PCL_HUMAN;              break;
+      case MOD_PAINSAW:           weapon = WP_PAIN_SAW;       class = PCL_HUMAN;              break;
+      case MOD_SHOTGUN:           weapon = WP_SHOTGUN;        class = PCL_HUMAN;              break;
+      case MOD_LASGUN:            weapon = WP_LAS_GUN;        class = PCL_HUMAN;              break;
+      case MOD_MDRIVER:           weapon = WP_MASS_DRIVER;    class = PCL_HUMAN;              break;
+      case MOD_CHAINGUN:          weapon = WP_CHAINGUN;       class = PCL_HUMAN;              break;
+      case MOD_PRIFLE:            weapon = WP_PULSE_RIFLE;    class = PCL_HUMAN;              break;
+      case MOD_FLAMER:            weapon = WP_FLAMER;         class = PCL_HUMAN;              break;
+      case MOD_FLAMER_SPLASH:     weapon = WP_FLAMER;         class = PCL_HUMAN;              break;
+      case MOD_LCANNON:           weapon = WP_LUCIFER_CANNON; class = PCL_HUMAN;              break;
+      case MOD_LCANNON_SPLASH:    weapon = WP_LUCIFER_CANNON; class = PCL_HUMAN;              break;
+      case MOD_GRENADE:           weapon = WP_GRENADE;        class = PCL_HUMAN;              break;
+      case MOD_ABUILDER_CLAW:     weapon = WP_ABUILD;         class = PCL_ALIEN_BUILDER0;     break;
+      case MOD_SLOWBLOB:          weapon = WP_ABUILD2;        class = PCL_ALIEN_BUILDER0_UPG; break;
+      case MOD_LEVEL0_BITE:       weapon = WP_ALEVEL0;        class = PCL_ALIEN_LEVEL0;       break;
+      case MOD_LEVEL1_CLAW:       weapon = WP_ALEVEL1;        class = PCL_ALIEN_LEVEL1;       break;
+      case MOD_LEVEL1_PCLOUD:     weapon = WP_ALEVEL1_UPG;    class = PCL_ALIEN_LEVEL1_UPG;   break;
+      case MOD_LEVEL2_CLAW:       weapon = WP_ALEVEL2;        class = PCL_ALIEN_LEVEL2;       break;
+      case MOD_LEVEL2_ZAP:        weapon = WP_ALEVEL2_UPG;    class = PCL_ALIEN_LEVEL2_UPG;   break;
+      case MOD_LEVEL3_CLAW:       weapon = WP_ALEVEL3;        class = PCL_ALIEN_LEVEL3;       break;
+      case MOD_LEVEL3_POUNCE:     weapon = WP_ALEVEL3;        class = PCL_ALIEN_LEVEL3;       break;
+      case MOD_LEVEL3_BOUNCEBALL: weapon = WP_ALEVEL3_UPG;    class = PCL_ALIEN_LEVEL3_UPG;   break;
+      case MOD_LEVEL4_CLAW:       weapon = WP_ALEVEL4;        class = PCL_ALIEN_LEVEL4;       break;
+      case MOD_LEVEL4_TRAMPLE:    weapon = WP_ALEVEL4;        class = PCL_ALIEN_LEVEL4;       break;
+      case MOD_LEVEL4_CRUSH:      weapon = WP_ALEVEL4;        class = PCL_ALIEN_LEVEL4;       break;
+      case MOD_MGTURRET:          weapon = WP_MGTURRET;       class = PCL_NONE;               break;
+      default:                    weapon = WP_NONE;           class = PCL_NONE;               break;
+    }
+    upgrade = UP_NONE;
+  }

Code:

+    // FIXME
+    targ->health = 1; // 1 hp hack to allow dretches to bite movers after their first use

[/dev/humancontroller]

This isn't ready for more serious inspection yet.

wrong.

You know that this is still nowhere near close to usable in trunk.

correct.

I do not believe that you believe this is remotely acceptable in trunkable code:

Code:

+  if( attacker && attacker->client && ( mod == MOD_MACHINEGUN || mod == MOD_PAINSAW ||
+        mod == MOD_SHOTGUN || mod == MOD_LASGUN || mod == MOD_MDRIVER ||
+        mod == MOD_CHAINGUN || mod == MOD_ABUILDER_CLAW || mod == MOD_LEVEL0_BITE ||
+        mod == MOD_LEVEL1_CLAW || mod == MOD_LEVEL2_CLAW || mod == MOD_LEVEL2_ZAP ||
+        mod == MOD_LEVEL3_CLAW || mod == MOD_LEVEL3_POUNCE || mod == MOD_LEVEL4_CLAW ||
+        mod == MOD_LEVEL4_TRAMPLE || mod == MOD_LEVEL4_CRUSH ) )

Code:

+    switch( mod )
+    {
+      case MOD_BLASTER:           weapon = WP_BLASTER;        class = PCL_HUMAN;              break;
+      case MOD_MACHINEGUN:        weapon = WP_MACHINEGUN;     class = PCL_HUMAN;              break;
+      case MOD_PAINSAW:           weapon = WP_PAIN_SAW;       class = PCL_HUMAN;              break;
+      case MOD_SHOTGUN:           weapon = WP_SHOTGUN;        class = PCL_HUMAN;              break;
+      case MOD_LASGUN:            weapon = WP_LAS_GUN;        class = PCL_HUMAN;              break;
+      case MOD_MDRIVER:           weapon = WP_MASS_DRIVER;    class = PCL_HUMAN;              break;
+      case MOD_CHAINGUN:          weapon = WP_CHAINGUN;       class = PCL_HUMAN;              break;
+      case MOD_PRIFLE:            weapon = WP_PULSE_RIFLE;    class = PCL_HUMAN;              break;
+      case MOD_FLAMER:            weapon = WP_FLAMER;         class = PCL_HUMAN;              break;
+      case MOD_FLAMER_SPLASH:     weapon = WP_FLAMER;         class = PCL_HUMAN;              break;
+      case MOD_LCANNON:           weapon = WP_LUCIFER_CANNON; class = PCL_HUMAN;              break;
+      case MOD_LCANNON_SPLASH:    weapon = WP_LUCIFER_CANNON; class = PCL_HUMAN;              break;
+      case MOD_GRENADE:           weapon = WP_GRENADE;        class = PCL_HUMAN;              break;
+      case MOD_ABUILDER_CLAW:     weapon = WP_ABUILD;         class = PCL_ALIEN_BUILDER0;     break;
+      case MOD_SLOWBLOB:          weapon = WP_ABUILD2;        class = PCL_ALIEN_BUILDER0_UPG; break;
+      case MOD_LEVEL0_BITE:       weapon = WP_ALEVEL0;        class = PCL_ALIEN_LEVEL0;       break;
+      case MOD_LEVEL1_CLAW:       weapon = WP_ALEVEL1;        class = PCL_ALIEN_LEVEL1;       break;
+      case MOD_LEVEL1_PCLOUD:     weapon = WP_ALEVEL1_UPG;    class = PCL_ALIEN_LEVEL1_UPG;   break;
+      case MOD_LEVEL2_CLAW:       weapon = WP_ALEVEL2;        class = PCL_ALIEN_LEVEL2;       break;
+      case MOD_LEVEL2_ZAP:        weapon = WP_ALEVEL2_UPG;    class = PCL_ALIEN_LEVEL2_UPG;   break;
+      case MOD_LEVEL3_CLAW:       weapon = WP_ALEVEL3;        class = PCL_ALIEN_LEVEL3;       break;
+      case MOD_LEVEL3_POUNCE:     weapon = WP_ALEVEL3;        class = PCL_ALIEN_LEVEL3;       break;
+      case MOD_LEVEL3_BOUNCEBALL: weapon = WP_ALEVEL3_UPG;    class = PCL_ALIEN_LEVEL3_UPG;   break;
+      case MOD_LEVEL4_CLAW:       weapon = WP_ALEVEL4;        class = PCL_ALIEN_LEVEL4;       break;
+      case MOD_LEVEL4_TRAMPLE:    weapon = WP_ALEVEL4;        class = PCL_ALIEN_LEVEL4;       break;
+      case MOD_LEVEL4_CRUSH:      weapon = WP_ALEVEL4;        class = PCL_ALIEN_LEVEL4;       break;
+      case MOD_MGTURRET:          weapon = WP_MGTURRET;       class = PCL_NONE;               break;
+      default:                    weapon = WP_NONE;           class = PCL_NONE;               break;
+    }
+    upgrade = UP_NONE;
+  }

ORLY.

i'll fix the naming issues later, when i'll have more time.

[/dev/humancontroller]

based on the reception of the range marker functionality, AMP is never going into the trunk.

[Aelita]

Just because you got code rejected due to style doesn't mean that the Trem devs have a vendetta against you. Maybe if you want to get something into trunk, you should follow their guidelines on style?