Why is only one value in my hash being changed?

Question

I'm making a simple RPG as a learning project, and am having an issue with part of the character creator.

This code should determine what skill string is assigned to player[:caste][:skill] and player[:sub][:skill], then increase each respective skill's value in player[:skills] by 2. This code should work regardless of what string is assigned to player[:caste][:skill] and player[:sub][:skill], as long as it is equal to player[:skills].to_s.

Currently, it is only applying the change to player[:skills][:endurance] but not player[:skills][:athletics].

player = {
  caste: {skill: "athletics"},
  sub: {skill: "endurance"},
  skills: {acrobatics: 0, athletics: 0, engineering: 0, endurance: 0, heal: 0, history: 0, influence: 0, insight: 0, magicka: 0, perception: 0, riding: 0, stealth: 0, streetwise: 0, thievery: 0},
}

player[:skills] = player[:skills].map do |skill, mod|
  [skill, (mod += 2 if skill.to_s == player[:caste][:skill])]
  [skill, (mod += 2 if skill.to_s == player[:sub][:skill])]
end.to_h

In other words, my code is returning the following player[:skills] hash:

skills: {acrobatics: 0, athletics: 0, engineering: 0, endurance: 2, heal: 0, history: 0, influence: 0, insight: 0, magicka: 0, perception: 0, riding: 0, stealth: 0, streetwise: 0, thievery: 0}

but I want it to return:

skills: {acrobatics: 0, athletics: 2, engineering: 0, endurance: 2, heal: 0, history: 0, influence: 0, insight: 0, magicka: 0, perception: 0, riding: 0, stealth: 0, streetwise: 0, thievery: 0}

Please let me know if there is a simpler way to do this. I've also tried the following:

player[:skills] = player[:skills].map do |skill, mod|
  [skill, (mod += 2 if skill.to_s == (player[:caste][:skill] || player[:sub][:skill]))]
end.to_h

which only affects the skill found in player[:caste][:skill].

Unrelated, but keeping everything in a hash may not be the best approach. It might be a good idea to encapsulate behavior like this into classes and avoid an arbitrary number of conditionals scattered throughout your code. — Dave Newton, Nov 17 '19 at 15:08
Thanks, Dave. I will keep that mind. In this project I'm storing data mostly in hashes and arrays to get better at using them. In a future project, I'll focus more on classes. — Richard, Nov 17 '19 at 15:30
Don't embed trailing `if` into the array. It's hard to decipher what you're doing and will insert nil if the test fails resulting in inconsistent data types you'll have to guard against later. — the Tin Man, Nov 17 '19 at 22:21

nPn · Accepted Answer · 2019-11-17T19:48:22.233

When I run your code I get this as the result.

{:acrobatics=>nil, :athletics=>nil, :engineering=>nil, :endurance=>2, :heal=>nil, :history=>nil, :influence=>nil, :insight=>nil, :magicka=>nil, :perception=>nil, :riding=>nil, :stealth=>nil, :streetwise=>nil, :thievery=>nil}

That's because map returns last statement executed. In addition you actually only set a value for skill when it's matches the sub skill otherwise, it is set to nil.

So whats happening in your code is that each iteration is returning the following which is the result of the last statement in the block passed into map.

[:acrobatics, nil]
[:athletics, nil]
[:engineering, nil]
[:endurance, 2]
[:heal, nil]
[:history, nil]
[:influence, nil]
[:insight, nil]
[:magicka, nil]
[:perception, nil]
[:riding, nil]
[:stealth, nil]
[:streetwise, nil]
[:thievery, nil]

The final result being an array that looks like this.

[[:acrobatics, nil], [:athletics, nil], [:engineering, nil], [:endurance, 2], [:heal, nil], [:history, nil], [:influence, nil], [:insight, nil], [:magicka, nil], [:perception, nil], [:riding, nil], [:stealth, nil], [:streetwise, nil], [:thievery, nil]]

Which is finally mapped to a new hash

{:acrobatics=>nil, :athletics=>nil, :engineering=>nil, :endurance=>2, :heal=>nil, :history=>nil, :influence=>nil, :insight=>nil, :magicka=>nil, :perception=>nil, :riding=>nil, :stealth=>nil, :streetwise=>nil, :thievery=>nil}

The reason you get all those nil's is because in your statements the result of the case were the if statement is not true is nil. For example:

[skill (mod += 2 if skill.to_s == player[:caste][:skill])]

will return [the_skill, nil] for the cases were skill.to_s == player[:caste][:skill] is not true

To see what's happening try this in irb.

x = 0
=> 0
x += 1 if false
=> nil
x += 1 if true
=> 1

You could get past that using something like this.

[skill, skill.to_s == player[:caste][:skill] ? mod + 2 : mod ]

or using the above example:

x = 0
=> 0
x =  false ? x + 1 : x 
=> 0
x =  true ? x + 1 : x
=> 1

The following modified version of your code should work.

player[:skills] = player[:skills].map do |skill, mod|
  [skill, skill.to_s == player[:caste][:skill] || skill.to_s == player[:sub][:skill] ? mod + 2 : mod ]
end.to_h

However, here is a slightly more verbose, but hopefully much easier to follow way to accomplish what you want to do and allows for added modifications in the future with out the code getting too confusing.

player = {
  caste: {skill: "athletics"},
  sub: {skill: "endurance"},
  skills: {acrobatics: 0, athletics: 0, engineering: 0, endurance: 0, heal: 0, history: 0, influence: 0, insight: 0, magicka: 0, perception: 0, riding: 0, stealth: 0, streetwise: 0, thievery: 0},
}

player_caste_skill = player[:caste][:skill]
player_sub_skill = player[:sub][:skill]
current_skills = player[:skills]
updated_skills = {}
current_skills.each_pair do |skill, prev_value| 
  new_value = prev_value 
  case skill.to_s
    when player_caste_skill, player_sub_skill
      new_value = prev_value + 2
    when "some_other_skill"  
      new_value = prev_value + 3
  end
  updated_skills[skill] = new_value
end
puts current_skills
puts updated_skills

Thanks for the well thought out explanation. I was able to follow both examples, but your first example using a ternary statement does not account for `player[:sub][:skill]`. I tried a few variations on: `[skill, skill.to_s == (player[:caste][:skill] || player[:sub][:skill]) ? mod + 2 : mod]` without success. Do you have any suggestions? I would like to do it this way if possible. — Richard, Nov 17 '19 at 19:28
I added a section that does what you want (at least I think it does). In the end it is not very different than what I posted at the end, but harder to follow — nPn, Nov 17 '19 at 19:53

score 1 · Answer 2 · answered Nov 17 '19 at 18:07

I'd set a default value (Hash#default) to the player[:skill] hash, just to avoid errors in case of missing key (it adds the key!!), allowing to add also a new key without the need to initialise to 0 each skill.

player[:skills].default = 0

Then scan the keys you need to increment in just one liner:

[:caste, :sub].each { |key| player.dig(key, :skill).to_sym.then { |skill| player[:skills][skill] += 2 } }

Thanks to the initialisation, your player can also be

player = {
  caste: {skill: "athletics"},
  sub: {skill: "endurance"},
  skills: {}
}

Returning a result like:

player #=> {:caste=>{:skill=>"athletics"}, :sub=>{:skill=>"endurance"}, :skills=>{:athletics=>2, :endurance=>2}}

Where:

player[:skills][:whatever] #=> 0

Consider also to empty `:caste` and `:sub` hash after the increment. I considered `:caste` and `:sub` having just one element or their value not being an array, otherwise you need to loop over them. — iGian, Nov 17 '19 at 18:13
There are a lot of things here that I didn't know about. Really compact as well. Thanks, iGian. — Richard, Nov 17 '19 at 19:44

score 1 · Answer 3 · answered Nov 17 '19 at 19:16

1

I would iterate through defined skills rather than through skill values.

player.
  map { |_, h| h[:skill] }.
  compact.
  map(&:to_sym).
  each { |skill| player[:skills][skill] += 2 }

Now player is updated accordingly, as you might check by exaimning player with p player or like.

answered Nov 17 '19 at 19:16

Aleksei Matiushkin

119,336
10
100
160

Thanks, Aleksei. What's the function of the periods (`.`) in your example? I haven't seen this before. – Richard Nov 17 '19 at 19:31

score 0 · Answer 4 · answered Nov 17 '19 at 14:58

0

Change the code to be something like this:

player = {
 caste: {skill: "athletics"},
 sub: {skill: "endurance"},
 skills: {acrobatics: 0, athletics: 0, engineering: 0, endurance: 0, heal: 0, 
 history: 0, influence: 0, insight: 0, magicka: 0, perception: 0, riding: 0, 
 stealth: 0, streetwise: 0, thievery: 0},
}

player[:skills] = player[:skills].map do |skill, mod|
  [skill, (mod += 2 if [player[:caste][:skill], player[:sub][:skill]].include? 
  (skill.to_s))]
end.to_h

The reason your code didn't work because map return the last line as a result for the current iteration, So in the athletics case the last line which is

[skill, (mod += 2 if skill.to_s == player[:sub][:skill])]

will be false which will be nil that's why only endurance case works.

hope it helps.

answered Nov 17 '19 at 14:58

Amr Adel

574
1
7
18

Thanks Amr. This is helpful. Is there a way to do this without setting every other value in `player[:skills]` to `nil`? I'd like each unaffected value to remain equal to `0`. – Richard Nov 17 '19 at 16:00
I tried the following: `player[:skills] = player[:skills].map { |skill, mod| [skill, ([player[:caste], player[:sub][:skill]].include?(skill.to_s) ? mod += 2 : mod = 0)] }.to_h` which causes the same problem as before, but manages to set every unaffected value to `0`. – Richard Nov 17 '19 at 16:09
Your code is fine you just forget to write it as `player[:caste][:skill]` you write it `player[:caste]` that is why it didn't give you the required output. Sorry for late response @Richard – Amr Adel Nov 18 '19 at 10:21

Why is only one value in my hash being changed?

4 Answers4