Review Request: Achievements 12: Add TasksStatistic class

[Laszlo Papp]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102700/#review6807
-----------------------------------------------------------



engine/tasksstatistic.h
<http://git.reviewboard.kde.org/r/102700/#comment6062>

    "This statistics class is used for a statistic where each defined event should only give the player a score increase once per game." ?



engine/tasksstatistic.h
<http://git.reviewboard.kde.org/r/102700/#comment6056>

    ...The Players should only get points if they visit a place the first time.
    
    or (I prefer this second one better)
    
    The player should only get points if a place is visited for the first time.
    
    Note: We do not show "chauvinism" this way, hehehe. Out of the joke: It does not matter if the player a male or female. Also "Here" is not needed since we are "here" by default. :p



engine/tasksstatistic.h
<http://git.reviewboard.kde.org/r/102700/#comment6059>

    Probably "@see markNotAccomplish" or so.
    
    I would prefer one more enter after the description in general. Everything in one bunch can be a bit harder to read than unneccesary with long parameter list, method description and so forth



engine/tasksstatistic.h
<http://git.reviewboard.kde.org/r/102700/#comment6060>

    mark(As)Accomplished term ? 
    
    I would personally prefer this method and the other mark(As)NotAccomplished one as a convenience method only, and I would probably do yet another method expecting a bool arguement as the second one and act accordingly. Not mandatory, can be added later, if the need arises more, but not a big task either to make it work.



engine/tasksstatistic.h
<http://git.reviewboard.kde.org/r/102700/#comment6058>

    probably "@see isAccomplished"



engine/tasksstatistic.h
<http://git.reviewboard.kde.org/r/102700/#comment6057>

    mark(As)NotAccomplished term ?



engine/tasksstatistic.h
<http://git.reviewboard.kde.org/r/102700/#comment6061>

    probably "@see mark(As)NotAccomplished, mark(As)Accomplished".



engine/tasksstatistic.cpp
<http://git.reviewboard.kde.org/r/102700/#comment6063>

    Just on a side note: I am not entirely sure about how performance critical this snippet is, but my experience is that it is rarely the situation JAVA-style iterator (in this special case: QListIterator) is significantly slower, but much more readable.
    
    If it is a very time-critical solution, pardon me.


- Laszlo Papp


On Sept. 25, 2011, 5:22 p.m., Felix Rohrbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102700/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2011, 5:22 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Description
> -------
> 
> Have a number of tasks in one statistic that can be accomplished in any order
> 
> 
> Diffs
> -----
> 
>   engine/CMakeLists.txt 6e13e5a 
>   engine/assets/other/statistics/statisticsasset.h f4eaa0f 
>   engine/assets/other/statistics/statisticsasset.cpp 69b74b1 
>   engine/tasksstatistic.h PRE-CREATION 
>   engine/tasksstatistic.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102700/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Felix Rohrbach
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/gluon/attachments/20110925/25bd717e/attachment-0001.html>