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

Viewing Issue Advanced Details [ Jump to Notes ] [ Wiki ] [ View Simple ] [ 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 Local or Remote Local
Status resolved   Operating System MacOSX
Projection none   Console or Tiles Tiles
ETA none Fixed in Branch 0.14 ancient branch Product Branch 0.14 ancient branch
  Product Version 0.14-a0-2989-gae9c01e
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.
Steps To Reproduce
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