After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 737145 - [PATCH] Remove Strategy enum.
[PATCH] Remove Strategy enum.
Status: RESOLVED FIXED
Product: iagno
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: iagno-maint
iagno-maint
Depends on:
Blocks:
 
 
Reported: 2014-09-22 21:25 UTC by Arnaud B.
Modified: 2014-09-25 03:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Remove Strategy enum. (4.23 KB, patch)
2014-09-22 21:25 UTC, Arnaud B.
reviewed Details | Review
Remove the Strategy enum. (4.75 KB, patch)
2014-09-23 17:27 UTC, Arnaud B.
none Details | Review
Remove the Strategy enum. (4.70 KB, patch)
2014-09-24 20:29 UTC, Arnaud B.
committed Details | Review

Description Arnaud B. 2014-09-22 21:25:01 UTC
Created attachment 286846 [details] [review]
Remove Strategy enum.

Knowing level is sufficient to decide what to do. Choosing a strategy at start and at an other place doing the job is the best way to make errors. Let’s remove Strategy enum.
Comment 1 Michael Catanzaro 2014-09-23 13:08:11 UTC
Review of attachment 286846 [details] [review]:

OK, but please say "difficulty_level" instead of simply "level" since that has nearly the same meaning as "depth" and would be quite confusing to have in the signature otherwise.
Comment 2 Arnaud B. 2014-09-23 17:27:38 UTC
Created attachment 286906 [details] [review]
Remove the Strategy enum.
Comment 3 Michael Catanzaro 2014-09-23 22:56:16 UTC
This doesn't apply on master. :( I think you've probably based it on the 3.14 branch again.
Comment 4 Arnaud B. 2014-09-24 20:29:08 UTC
Created attachment 287010 [details] [review]
Remove the Strategy enum.

This batch was build on top of the “Faster history” série, but now it can’t apply anymore because of the last threading patch. ^^ Here is an update.

By the way, I don’t understand the:
    if (depth == 0 || !move_pending)
        return calculate_heuristic (g);
Is it really completely necessary to launch calculate_heuristic() if the move has been cancelled?
Comment 5 Michael Catanzaro 2014-09-25 03:14:19 UTC
(In reply to comment #4)
> This batch was build on top of the “Faster history” série, but now it can’t
> apply anymore because of the last threading patch. ^^ Here is an update.

That definitely wasn't the problem; I tried to apply this one first, figuring I'd rather fix a small conflict in my own patch than in yours.

> By the way, I don’t understand the:
>     if (depth == 0 || !move_pending)
>         return calculate_heuristic (g);
> Is it really completely necessary to launch calculate_heuristic() if the move
> has been cancelled?

It shouldn't matter what gets returned, or even if we have that check at all.
Comment 6 Michael Catanzaro 2014-09-25 03:14:31 UTC
Attachment 287010 [details] pushed as 9cf1e50 - Remove the Strategy enum.