Main | My View | View Issues | Change Log | Wiki | Tavern | News
  

Viewing Issue Simple Details [ Jump to Notes ] [ Wiki ] [ View Advanced ] [ Issue History ] [ Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0008235 [DCSS] Bug Report minor have not tried 2014-03-06 02:01 2014-03-06 04:25
Reporter floatingatoll View Status public  
Assigned To neil
Priority normal Resolution done  
Status resolved   Product Branch 0.14 ancient branch
Summary 0008235: Crosstraining green '*' not shown on Throwing when Slings is also training
Description When training both Throwing and Slings, the green crosstraining '*' as indicated by the Skills legend is not shown on the Throwing skill, even though Slings has the expected green text highlight and -1 +4 shown.

Hill Orc, Warrior, fresh game started on 0.14-a0-2989-gae9c01e compiled with make APPLE_GCC=y NO_PKGCONFIG=y CONTRIB_SDL=y TILES=y.
Additional Information
Tags No tags attached.
Attached Files png file icon Screen Shot 2014-03-05 at 4.58.16 PM.png [^] (63,881 bytes) 2014-03-06 02:01


? file icon 8235-skill_menu.patch [^] (610 bytes) 2014-03-06 03:17 [Show Content]

- Relationships

-  Notes
(0025474)
floatingatoll (reporter)
2014-03-06 02:52

 I am also unable to see any green '*' marking any skill (except the crosstrained skill itself) with OS X tiles for sword skills on 0.13.1-28-g3517093, 0.13.0 (429a20e13a368), 0.12.0 (a35e613580ded6).

I also tested with console, same issue.
(0025475)
floatingatoll (reporter)
2014-03-06 03:17
edited on: 2014-03-06 03:18

 I debugged further; this issue is due to "if (crosstrain_other(m_sk, show_all))" returning false, and thus not placing the green * for the skill responsible for the crosstraining.

This change fixes the skill menu to correctly show the green *:

     for (unsigned int i = 0; i < crosstrain_skills.size(); ++i)
         if (you.skill(crosstrain_skills[i], 10, true)
-            <= you.skill(sk, 10, true) - CROSSTRAIN_THRESHOLD
-           && (you.skills[crosstrain_skills[i]] > 0 || show_zero))
+            <= you.skill(sk, 10, true) - CROSSTRAIN_THRESHOLD)
         {
             return true;


And that's probably a bad idea, since there's actually a show_all boolean specifically for this:

      const bool show_all = skm.get_state(SKM_SHOW) == SKM_SHOW_ALL;


Which, if I read this right, is only enabled when the skill menu is set to show *all* skills, not just the ones available for training.

And sure enough, pressing '*' to change to "Show: all" results in the green * being displayed correctly.

So, when crafting an entry for the skill menu, the _bonus functions need to be provided show_all = true *rather than* whatever the skill menu's SHOW_ALL value is.

And so these two changes repair the skill menu to correctly indicate the source of crosstraining, without altering any of the actual skill code.

@@ -349,12 +349,12 @@ void SkillMenuEntry::set_aptitude()
-    if (antitrain_other(m_sk, show_all))
+    if (antitrain_other(m_sk, true))
...
-    else if (crosstrain_other(m_sk, show_all))
+    else if (crosstrain_other(m_sk, true))


(0025478)
floatingatoll (reporter)
2014-03-06 03:48

 bh helped me repro this on webtiles, and |amethyst notes that it requires the source of the crosstraining (Slings, for instance) to be 0 (or less than 1, I think?).

antitrain_other() and crosstrain_other() are only used by the skillmenu, so they could be altered to do the right thing if needed.

my idea of adding , true breaks if Throwing is 2.0 and Slings is not available to train.

I tried to do this:

~            && (you.skills[crosstrain_skills[i]] > 0 || show_zero || you.skills[crosstra
in_skills[i]].can_train))


But it turns out that you can't just call methods on a skill integer value (wishful thinking, fail).
(0025482)
floatingatoll (reporter)
2014-03-06 04:00

 The addition of a you.can_train test:

&& (you.skills[crosstrain_skills[i]] > 0 || you.can_train[crosstrain_skills[i]] || show_z
ero))


Fixes the issue correctly for every test I've come across so far (Long 2.1, Short 0.0, carrying or not a short sword).

|amethyst is working out a patch that moves this crosstrain_other() function, which is only used by the skill menu, into the skill_menu.cc code to make better use of the 'should I show a skill or not' function for this. That's beyond my skill level, but I agree anyways :)
(0025484)
neil (administrator)
2014-03-06 04:25

 Fixed in trunk (0.14-a0-2993-g1142d79), thanks! I'm not sure if my refactoring really was an improvement though :/

- Issue History
Date Modified Username Field Change
2014-03-06 02:01 floatingatoll New Issue
2014-03-06 02:01 floatingatoll File Added: Screen Shot 2014-03-05 at 4.58.16 PM.png
2014-03-06 02:52 floatingatoll Note Added: 0025474
2014-03-06 03:17 floatingatoll Note Added: 0025475
2014-03-06 03:17 floatingatoll File Added: 8235-skill_menu.patch
2014-03-06 03:18 floatingatoll Note Edited: 0025475
2014-03-06 03:48 floatingatoll Note Added: 0025478
2014-03-06 04:00 floatingatoll Note Added: 0025482
2014-03-06 04:25 neil Note Added: 0025484
2014-03-06 04:25 neil Status new => resolved
2014-03-06 04:25 neil Fixed in Branch => 0.14 development branch
2014-03-06 04:25 neil Resolution open => done
2014-03-06 04:25 neil Assigned To => neil

Mantis 1.1.8[^]
Copyright © 2000 - 2009 Mantis Group
Powered by Mantis Bugtracker