This is a list of all comments for SWGEMU-557.

Review Summary: No summary

General Comment by Loshult on 17 March 2013, 23:15 
http://www.swgemu.com/fisheye/cru/SWGEMU-557#c1718

  We have started to use Gerrit for reviews instead of Fishey.
  Please follow the instructions on the following page:
  https://www.assembla.com/spaces/swgemu/wiki/Git_Gerrit_and_Jenkins
  and push your patch to Gerrit instead.


----------------------------------------
File: MMOCoreORB/src/server/zone/managers/combat/CombatManager.cpp
Revision Comment by dannuic on 27 January 2013, 10:12 
http://www.swgemu.com/fisheye/cru/SWGEMU-557#c1379

  This is an odd way to do this. Why don't you just use specific
  skillmods that go above the cap? None of the skillmods that go
  over the cap are the same as the ones that stay under it (which
  I'm certain is why they were special in the first place in the
  live code). Directly using the buffs is a little hacky and
  causes lots of room to apply bonuses twice due to small code
  errors. Using the same system for both pre-and post-cap mods
  would be preferable.

  Reply by Yog-Sothoth on 27 January 2013, 16:26
  > Just so we don't change the skillmod, as you've already
  > pointed in my previous fix.
  > This one will make sure the skillmod used isn't a problem.
  > 
  > You replied to one of my changes as saying that one of the
  > changes should stay melee_defense.
  > I don't see how this change doesn't suit that need.
  > 
  > I've also commented on where the buff bonus is removed, so we
  > can added them after the cap.
  > 
  > To be honest, I can't see why this isn't a proper one.
  > I do exactly as you've asked, by using a array of constants.
  > I just made sure that the name of the skillmod doesn't
  > matter, as long as the buff is in the array.

    Reply by dannuic on 27 January 2013, 21:01
    > No, I said a constant array of strings, not an array of
    > constants for this situation. This is very hacky, which is
    > why it's not a good fix. Which buff has a skillmod of
    > melee_defense that should go above the cap?

      Reply by Yog-Sothoth on 27 January 2013, 22:10
      > All of them?
      > I've considered the other way, but preferred this one
      > over it.
      > I don't see how this is hacky.
      > We have an array that defines buffs that should provide
      > the bonus over the cap. We check if the player has such
      > buffs, remove them from the initial value. If we still
      > have a value above 125, we cap at that value, and reapply
      > all the bonus from buffs after that. If not, we just
      > reapply the value from the buffs without any further
      > change.
      > To me it looks clean, simple, and we don't need to change
      > any skillmod bonus, they will work like they should.
      > 
      > One of the things that you've mentioned in the other
      > review was that the defense bonus should appear in the
      > skillbox, this will do that, while keeping the buff value
      > over the cap.
      > 
      > Could you please point out the error of my logic?

        Reply by dannuic on 27 January 2013, 22:40
        > Because you are hard-coding individual buffs into
        > combat manager instead of selecting more generic
        > strings that can go over the defense cap. The strings
        > can be renamed in the skills (except I think the food
        > one, but that's unique anyway), but your error in
        > renaming them before was modifying them in situ instead
        > of just having them called something else from the
        > beginning.

          Reply by Yog-Sothoth on 27 January 2013, 22:48
          > Shouldn't they be hard-coded anyway? Since they are
          > the only exceptions to the rule.
          > 
          > And shouldn't renaming them to something else
          > prevents them from being properly displayed in the
          > skillbox (if needed)?

----------------------------------------
File: MMOCoreORB/src/server/zone/objects/creature/CreatureObjectImplementation.cpp
Revision Comment by dannuic on 27 January 2013, 10:15 
http://www.swgemu.com/fisheye/cru/SWGEMU-557#c1380

  private_defense is not already getting added generically? It
  should be affecting all defenses, there shouldn't be a reason
  to break it out to the individual primary defenses.

  Reply by Yog-Sothoth on 27 January 2013, 16:25
  > private_defense is added after the cap, this is to make sure
  > stun is applied BEFORE the hard cap.


---

ID: SWGEMU-557 http://www.swgemu.com/fisheye/cru/SWGEMU-557

Title: Stun State and Primary Defense Buffs
Statement of Objectives:
As requested, here is the individual patch.

Changes:

Stun State penalty is applied before the Primary Defense hardcap.
Stun State penalty will no longer be ignored by Creatures/NPCs.
Foods and Squad Leader bonus will be applied after the hardcap.

NOTE: Added an array of constants for buffs that should go above the hard cap, as requested by dannuic.

State: Review

Author: Yog-Sothoth
Moderator: TheAnswer
Reviewers: (8 active, 0 completed*)
     Itac
     Loshult
     cRush
     dannuic
     oru
     Kyle
     elpete
     bluree