Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better Lua string-parsing #1

Open
anaseskyrider opened this issue Feb 22, 2021 · 1 comment
Open

Better Lua string-parsing #1

anaseskyrider opened this issue Feb 22, 2021 · 1 comment

Comments

@anaseskyrider
Copy link

anaseskyrider commented Feb 22, 2021

The current method you use to extract the substrings of the dice notation in Lua is very inefficient (and pollutes the namespace with globals). This stuff is what pattern-matching is for and reduces most of the work to just a couple lines of code and some checks. Over 1,000,000 iterations with my method, I saw improved execution speeds of around 270%.

Getting a new math seed every time the roll is called is also only useful for command-line executions of this script in quick-succession, and is not usable (and counter-productive!) for video game applications where the script doesn't reseed with every execution. I recommend replacing that with a bool.

function roll(diceString, reset)
  -- get dice info
  local pattern = "^(%d*)[Dd](%d+)([-+/*]?)([+-]?%d*)"
  local dice,size,op,mod = diceString:match(pattern)
  dice = (dice=="" and 1) or tonumber(dice)
  size = (size=="" and 1) or tonumber(size)
  op = (op=="" and nil) or op -- allows for easy checks with "if op then ... end"
  mod = (mod=="" and nil) or tonumber(mod)

  if reset then
    -- the os.time() is commonly cited as a good way to randomly
    --    seed randomness. However, die rolls aren't random in
    --    this case when running the script multiple times in one
    --    second. FOund this seeding technique on:
    --    https://www.reddit.com/r/lua/comments/1b5qg3/need_help_with_truly_random_number_generator/c940pyj/
    math.randomseed(os.time() + tonumber(tostring({}):sub(8)))
  end

  -- start rolling
  local result = 0
  for i=1, dice do
    result = result + math.random(1, size)
  end

  -- apply modifiers to total
  if op and mod then
    -- could add elifs for * and /
    if op == "+" then
      result = result+mod
    elseif op == "-" then
      result = result-mod
    end
  end
  return result
end
@brianherbert
Copy link
Owner

Thanks for the detailed write up! Totally unexpected in this repo and appreciate the critique for my first lua script. If you'd like to submit a pull request, I'll happily incorporate your changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants