Reputation System 1.0

[Moderator]

12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.

15:50, 17th Feb 2013
Magtheridon96:

• wanttoregister should be WANT_TO_REGISTER because that's how constant booleans are written in JASS.
• All your struct member names should be camelCased. (reptypename -> repTypeName, repname -> repName, etc...)
  This is JASS convention and it would make them easier to read.
• I think it would be more suitable to make a single rep change event and have a boolean as event response data to tell you whether it's increasing or decreasing.
  This way, you satisfy people who just want to detect whether the rank is equal to something or not while still keeping the people who want specific code to run for each type of change happy (Optional change, just throwing in my opinion)
• Code

  if saver[this][GetUnitUserData(u)] == 0 or u == null then
      return false
  else
      set saver[this][GetUnitUserData(u)] = 0
      set hunit[this] = hunit[this] - 1
      set origown[this] = null
      return true
  endif

  ->
  

  if saver[this][GetUnitUserData(u)] == 0 or u == null then
      return false
  endif
  
  set saver[this][GetUnitUserData(u)] = 0
  set hunit[this] = hunit[this] - 1
  set origown[this] = null
  
  return true

  You can do similar things for most of your functions as well.
• The documentation should be updated because at least one of the API elements is incorrectly listed (struct reputation -> struct Reputation)
• Can't the API listing just be the function definitions with descriptions that go under to avoid annoying scrolling? :/
  
  Example

  /*
  *   static method create takes string name returns Reputation
  *       - Description.
  *
  *   static method createEx takes string name, integer something returns Reputation
  *       - Description.
  *
  */

  I think the above is better because it would assure that the entire API listing is uniform, clean and easy to understand.
  (Also, not everyone knows the func(args, args2, args3) notation
• remove should be named destroy to comply to Struct method naming standards :/
  Whenever a user calls something that is not "destroy", he is going to expect the instance to remain unrecycled.
• Player(15) and Player(12) should probably be configurable in the globals block at the top.
• In the addToFaction method, you would benefit (readability and efficiency) if you were to cache the unit user data into a local integer.
• In the addToFaction method, you can discard the "else" and return false at the end of the function.

I think this covers most things :v

 

[CoLd Bon3]

Uhhh... i was waiting for a resource from you for a looong time, edo! But i wanna see how it works ingame.. When i test the map nothing happens. Just fog of war appears. Is it a bug, do i have to type something or you just simply didn't put anything in there, just code. I don't know jass at all so i don't know what to do.
You can consider giving an example in the next version of the system (if there isn't in that one).

 

[aaron]

finally something original. I really do hope this works.

 

[CHA_Owner]

method addToFaction takes unit u returns boolean
            
            //double save protection
            
            if saver[this][GetUnitUserData(u)] != GetUnitUserData(u) and u != null then
                set saver[this][GetUnitUserData(u)] = GetUnitUserData(u)
                set origown[GetUnitUserData(u)] = GetOwningPlayer(u)
                set hunit[this] = hunit[this] + 1
                call .update()
                return true
            else
                return false
            endif
        endmethod

This has way too many calls in it set the GetUnitUserData(u) into a variable then use the variable in place of all the calls to increase efficiency.

EDIT*

Other than that 1 fix this system looks great so ill vote for approval 4/5 for originality once coding is cleaned up will change to 5/5.