[zookeeper]

Tired of being limited to 4-quadrant shields? Me too.

What this feature is about is allowing one to define an arbitrary number of shield quadrants/segments and their placement. The mechanics are simple: you place special points with $special=shieldpoint in the model file, with each point representing one shield segment. Then, when the ship's shield takes a hit, the segment which gets hit is the one which's point is closest. Can be used with shield meshes, surface shields, the new auto spread shields, you name it.

There's a few limitations; most importantly, the game cannot auto-generate shield icons for these so shield icons are necessary. This also means that the mini target shield indicator under the reticle won't work right (because it uses the same icon for all ships), although that could be fixed by letting each ship define their own mini target shield icon as well. Also, there's still only 4 shield augmentation keys available, so player ships can't really use more segments than that (unless you circumvent that limitation yourself). Finally, the segments still need to be representable in the shield icons so that can put some practical limitations on the shield layouts on some ships.

So, what you need to use the feature:

1. Place special points in the model file. Name doesn't matter and neither does radius, only the placement. Add the $special=shieldpoint property.
2. Make a corresponding shield icon. The order of each individual shield segment frame needs to be the same as the order of the special points.
3. Give the ship class the "model point shields" flag.
4. If it's a player ship, you need to re-map the shield augmentation controls. For that you should use the following in the ships table:

Code: [Select]

...
$Shields: 100
$Model Point Shield Controls: ( "front" "rear" "none" "none" )
$Power Output: 2.0
...
What that example means is that the first segment (again based on the ordering from the model file) should be augmented by the "augment forward shields" key, the second segment augmented by the "augment rear shields" key, and the two other controls should do nothing. So, you'd use this for a ship which has 2 shield segments, front and rear.

That can be a bit confusing so let's have another example, for a ship with one a rear left and rear right segments as well as a front segment (in that order). That'd go like this:

Code: [Select]

$Model Point Shield Controls: ( "left" "right" "front" "none" )
So the first segment (left rear) would be augmented with the "augment left shields" key, the second segment (rear right) with the "augment right shields" and the third one (front) with the "augment forward shields" key. There is no fourth segment so the last one remains "none".

Huge patch attached, and also found here (since the attachment will be eaten sooner or later). I'll maybe post some visualizations/screenshots a bit later.

EDIT: Updated the patch.
EDIT: Updated the patch again.

[attachment deleted by ninja]

[zookeeper]

Here's a basic example of what you can do with these. Let's say you have an asymmetric ship like this little transport from FotG:



The default four quadrants don't work that nicely here (except for the tail), so it'd be much nicer if instead the ship was divided into either two (left/right) or three (left/right/rear) shield segments. Let's go with three and set up three shieldpoints like this:



Since it can be kinda hard to tell where exactly the borders between the segments will end up being, I wrote myself a 3ds Max script with which I can produce simple visualizations of the area each shield segment will cover, so it's easy to fine-tune the locations precisely. Here's what the above shieldpoints will result in:



Green is left, red is right, blue is rear. It looks pretty good, especially compared to the default 4-quadrant layout which would look like this:



The default's arguably not terribly terrible, but it's not great either, especially in the middle. If there's a subsystem placed in the middle between the main pods, it'd be particularly annoying to try to hit it when any given shot could be hitting pretty much any of the quadrants.

[fightermedic]

wait, this will allow us to create ships with only one shield segment?

[zookeeper]

wait, this will allow us to create ships with only one shield segment?

Yes. Although I think that's one case I haven't tested yet.

[Rodo]

This looks way interesting, I recon a few possibilities opening on the modding side.
Kudos on the great work!

[Droid803]

Could this possibly be extended to *more* than 4-segment shields?
aka. a 6-section shield.

EDIT: seems like its possible, just not for player ships.

[zookeeper]

Could this possibly be extended to *more* than 4-segment shields?
aka. a 6-section shield.

EDIT: seems like its possible, just not for player ships.

Yeah, it's possible, and we'll likely have up to 7-segment shields in FotG. You can use them even on player ships, but there will still only be 4 shield augmentation controls so it wouldn't really work right except in specific circumstances (such as if shield augmentation is turned off).

[AndrewofDoom]

How will the game distribute the shield points? I'm assuming evenly across all points, but I'm making sure. That being said, I've been looking for a universal shield vs the quadrant shield setup FS has, and by using only one shield point, looks like I'll be able to do just that.

Second, will this work with multiplayer?

[zookeeper]

How will the game distribute the shield points? I'm assuming evenly across all points, but I'm making sure.

I'm afraid you need to rephrase, as I'm not sure what you mean.

Second, will this work with multiplayer?

I did update the quadrant handling in the multiplayer code, but it's untested.

[MatthTheGeek]

How will the game distribute the shield points? I'm assuming evenly across all points, but I'm making sure.

I'm afraid you need to rephrase, as I'm not sure what you mean.

I think he means "health points" for shields, ie the $Shields value in the tables.

I am honestly not sure what is the base behaviour for that one.

[zookeeper]

I think he means "health points" for shields, ie the $Shields value in the tables.

I am honestly not sure what is the base behaviour for that one.

Oh, right. Yes, evenly across all points, just as with stock quadrants.

[niffiwan]

Ok, so I've finished a code review on the patch 

I updated to the patch to apply to r10084, and added two fixes as part of that:
1) Minor update to code/object/object.cpp:148 (new code added as part of converting struct object to a class)
2) Minor update to code/ship/ship.cpp:2524 (gcc failed to compile, changed the SCP_string to the const char * in the Warning())

(both these are included in the attached patch)

Also note that I haven't run any tests with models that include the new shieldpoints, I don't know my way around PCS2 well enough to add them, and I figure that the FotG team has already tested this extensively.

Now, my questions/comments about the patch:

(1)
hull_hit_index used to be set to the same value as the number of quadrants. (i.e. #define HULL_HIT_OFFSET 4)
With the patch it's set to one less than the number of quadrants.  I think this means that the last point shield quadrant will be treated as being the hull? e.g. for flashing the escort list, or the ship shields icon?  (see the linked mission for a test case)
This would also affect shield_info_reset() / NUM_SHIELD_HIT_MEMBERS (originally 5)

(2)
It could be more robust to move "Assert(objp->type == OBJ_SHIP);" to be prior to creating *shipp in hud_augment_shield_quadrant()?

(3)
I don't think the added comparison here will have any effect:
hud_shield_show_mini() ~line 409

Code: [Select]

- if ( objp->shield_quadrant[Quadrant_xlate[i]] < 0.1f ) {
+ if ( Quadrant_xlate[i] > objp->n_quadrants || objp->shield_quadrant[Quadrant_xlate[i]] < 0.1f ) {
Quadrant_xlate[ i ] will never be greater than objp->n_quadrants:
n_quadrants = 1, MAX Quadrant_xlate = 1
n_quadrants = 2, MAX Quadrant_xlate = 1
n_quadrants = 3, MAX Quadrant_xlate = 2
n_quadrants = 4, MAX Quadrant_xlate = 3

I guess the intent is to avoid accessing beyond the vector size, perhaps it should be >= instead of >?

(4)
In HudGaugeShield::showShields(), it seems that ships with SIF2_SHIELD_POINTS will draw a shield quadrant/section even if it has been reduced to zero?  (I'd guess that it would be faint, but still there)
i.e. there's no equivalent to this for ships with SIF2_SHIELD_POINTS

Code: [Select]

- if ( objp->shield_quadrant[Quadrant_xlate[i]] < 0.1f ) {
+ if ( !(sip->flags2 & SIF2_SHIELD_POINTS) && objp->shield_quadrant[Quadrant_xlate[i]] < 0.1f ) {
  continue;
  }

(5)
I think the LUA shields type will be partly broken for any ships that use model shield points. It's hardcoded to expect 4 quadrants.  I'm not sure of the best way to fix this, given that the quadrants are all hardcoded in the big LUA enumerations array.
ade_obj<object_h> l_Shields("shields", "Shields handle");


Anyway, let me know what you think about these questions/comments.


New patch: http://www.mediafire.com/download/nk4bckkk1rj8q22/model_point_shields_v2-svn.patch (apologies, my version of SVN is old and doesn't alphabetically order the files in the patch)

Test mission for point 1: http://www.mediafire.com/view/yenhdxfq9o86y1s/hull_idx_tst.fs2 (with thanks to Yarn for the original mission)

[zookeeper]

Cool.

I had been waiting to apply a certain little cosmetic fix to hudtarget.cpp before posting an updated patch, but since you're on this already, check the first post for the update; it's applied to r10092, and I think all issues except the Lua shields one are now covered, I think.

Ok, so I've finished a code review on the patch 

I updated to the patch to apply to r10084, and added two fixes as part of that:
1) Minor update to code/object/object.cpp:148 (new code added as part of converting struct object to a class)
2) Minor update to code/ship/ship.cpp:2524 (gcc failed to compile, changed the SCP_string to the const char * in the Warning())

Yeah, I had applied 2) already.

(both these are included in the attached patch)

Also note that I haven't run any tests with models that include the new shieldpoints, I don't know my way around PCS2 well enough to add them, and I figure that the FotG team has already tested this extensively.

Yes, and it'll receive a whole lot more testing once more of our ships get converted to the shieldpoints system.

Now, my questions/comments about the patch:

(1)
hull_hit_index used to be set to the same value as the number of quadrants. (i.e. #define HULL_HIT_OFFSET 4)
With the patch it's set to one less than the number of quadrants.  I think this means that the last point shield quadrant will be treated as being the hull? e.g. for flashing the escort list, or the ship shields icon?  (see the linked mission for a test case)
This would also affect shield_info_reset() / NUM_SHIELD_HIT_MEMBERS (originally 5)

Yes, fixed in the updated patch.

(2)
It could be more robust to move "Assert(objp->type == OBJ_SHIP);" to be prior to creating *shipp in hud_augment_shield_quadrant()?

Very true, fixed in the updated patch.

(3)
I don't think the added comparison here will have any effect:
hud_shield_show_mini() ~line 409

Code: [Select]

- if ( objp->shield_quadrant[Quadrant_xlate[i]] < 0.1f ) {
+ if ( Quadrant_xlate[i] > objp->n_quadrants || objp->shield_quadrant[Quadrant_xlate[i]] < 0.1f ) {
Quadrant_xlate[ i ] will never be greater than objp->n_quadrants:
n_quadrants = 1, MAX Quadrant_xlate = 1
n_quadrants = 2, MAX Quadrant_xlate = 1
n_quadrants = 3, MAX Quadrant_xlate = 2
n_quadrants = 4, MAX Quadrant_xlate = 3

I guess the intent is to avoid accessing beyond the vector size, perhaps it should be >= instead of >?

Yes, it was an out of bounds check which didn't really make sense. Fixed and done a bit differently in the updated patch (now it simply checks if i >= DEFAULT_SHIELD_SECTIONS, because the Quadrant_xlate array is specific to the default shields).

(4)
In HudGaugeShield::showShields(), it seems that ships with SIF2_SHIELD_POINTS will draw a shield quadrant/section even if it has been reduced to zero?  (I'd guess that it would be faint, but still there)
i.e. there's no equivalent to this for ships with SIF2_SHIELD_POINTS

Code: [Select]

- if ( objp->shield_quadrant[Quadrant_xlate[i]] < 0.1f ) {
+ if ( !(sip->flags2 & SIF2_SHIELD_POINTS) && objp->shield_quadrant[Quadrant_xlate[i]] < 0.1f ) {
  continue;
  }


Yes, good catch. Fixed in the updated patch.

(5)
I think the LUA shields type will be partly broken for any ships that use model shield points. It's hardcoded to expect 4 quadrants.  I'm not sure of the best way to fix this, given that the quadrants are all hardcoded in the big LUA enumerations array.
ade_obj<object_h> l_Shields("shields", "Shields handle");

Yeah, I still have to deal with that. Doesn't look terribly complicated though, but I'd still rather do that afterwards.

EDIT: Oh, one more thing. When merging I was unsure as to what to do in object.cpp:132, so for now I just removed that line. I presume that shield_quadrant.clear() would be the right thing to do there?

[niffiwan]

I'm glad the review was helpful   The new patch seems to take of all my points, except LUA (as you indicated this will be fixed in future) and this one...

(1)
hull_hit_index used to be set to the same value as the number of quadrants. (i.e. #define HULL_HIT_OFFSET 4)
With the patch it's set to one less than the number of quadrants.  I think this means that the last point shield quadrant will be treated as being the hull? e.g. for flashing the escort list, or the ship shields icon?  (see the linked mission for a test case)
This would also affect shield_info_reset() / NUM_SHIELD_HIT_MEMBERS (originally 5)

Yes, fixed in the updated patch.

This still seems to be an issue in my test mission.  Maybe this:

Code: (void shield_info_reset(object *objp, shield_hit_info *shi)) [Select]

+ shi->members = objp->n_quadrants;

Should be this?

Code: (void shield_info_reset(object *objp, shield_hit_info *shi)) [Select]

+ shi->members = objp->n_quadrants + 1;


EDIT: Oh, one more thing. When merging I was unsure as to what to do in object.cpp:132, so for now I just removed that line. I presume that shield_quadrant.clear() would be the right thing to do there?

Yeah, shield_quadrant.clear() is probably the right thing to do here.

[zookeeper]

I'm glad the review was helpful   The new patch seems to take of all my points, except LUA (as you indicated this will be fixed in future) and this one...

(1)
hull_hit_index used to be set to the same value as the number of quadrants. (i.e. #define HULL_HIT_OFFSET 4)
With the patch it's set to one less than the number of quadrants.  I think this means that the last point shield quadrant will be treated as being the hull? e.g. for flashing the escort list, or the ship shields icon?  (see the linked mission for a test case)
This would also affect shield_info_reset() / NUM_SHIELD_HIT_MEMBERS (originally 5)

Yes, fixed in the updated patch.

This still seems to be an issue in my test mission.  Maybe this:

Code: (void shield_info_reset(object *objp, shield_hit_info *shi)) [Select]

+ shi->members = objp->n_quadrants;

Should be this?

Code: (void shield_info_reset(object *objp, shield_hit_info *shi)) [Select]

+ shi->members = objp->n_quadrants + 1;


Right. I had actually done that already, but it was uncommitted and thus didn't end up in the patch.

EDIT: Oh, one more thing. When merging I was unsure as to what to do in object.cpp:132, so for now I just removed that line. I presume that shield_quadrant.clear() would be the right thing to do there?

Yeah, shield_quadrant.clear() is probably the right thing to do here.

Great. I'll make some finishing touches and post one more (hopefully final) patch in the next day or two. There were a few trivial changes needed for area damage and beams to work right that I had forgotten to commit, too.

[zookeeper]

Aaand here we go (updated the first post). Unless anything further crops up, I'll consider this final.

[niffiwan]

Looks good, go ahead and commit it

[zookeeper]

Committed. I only did a cosmetic naming tweak (r10133), an include fix (r10134, dunno how it had managed to slip through) and an initialization problem Goober spotted (r10137) since the last patch.

I'll update the ships.tbl wiki ASAP.

EDIT: Done. I (also) made a new page with an overview and instructions, although it lacks pictures.