zetter / multicrosser

Multiplayer Crosswords
https://multicrosser.chriszetter.com
15 stars 7 forks source link

Issue when using crosswords larger than 20x20 #42

Open shuesken opened 2 years ago

shuesken commented 2 years ago

First of all, thank you for this software! I have been self-hosting a slightly modified version using NYT crosswords to play with my friends and it has been a wonderful addition to the sometimes dreary pandemic life.

We were stumped by an occasional bug that made it impossible for new joiners to see existing crosswords. It's based in app/channels/moves_channel.rb:

  def subscribed
    stream_from(channel_name)
    data = redis.hgetall(channel_name)

    grid = Array.new(20) { Array.new(20) }

    data.each {|k, v|
      x, y = k.split('-')
      next if x.nil? or y.nil?
      next unless x.to_i.in?(0..20) && y.to_i.in?(0..20)
      grid[x.to_i][y.to_i] = v
    }

    transmit({'initialState': grid})
  end

This code will fail if x == 20. To make it work, the line should be next unless x.to_i.in?(0..19) && y.to_i.in?(0..19).

This only happens in crosswords larger than 20x20. In general, this function will not work properly for crosswords of that size. In my version, I fixed this by using grid = Array.new(50) { Array.new(50) } and next unless x.to_i.in?(0..49) && y.to_i.in?(0..49). However, it might be good to make the grid size dynamic anyway, or at least base it on a MAX_CROSSWORD_SIZE constant.

This is not a bug that will affect your current deployment, I'm only mentioning it because you expressed interest in how to support other crosswords in another issue.

Thank you again for creating this wonderful piece of software! Please know that you've enabled hours of joy :)

zetter commented 2 years ago

Hi @shuesken, It's great to hear how you've been enjoying crosswords.

Since I'm not working on this project any more I don't intend to fix this, I'll leave this report open in case it's helpful for others using the code.