Conversation
Enumerations can hold both a number and a string representation. This simplifies a little bit the procedures to get the atoms
Reordered the code a little bit to prevent circular dependencies after deleting the forward declarations. This resulted in some procedures now living in `events.nim`.
Just looked for repeated patterns across the code and moved some of it into separate procedures.
|
Thanks a lot! I've been wanting to refactor the code for a while but haven't gotten around to doing it. Everything here looks good, thanks! |
|
I got a chance to test this more extensively, the WM itself still works just fine, but the |
|
Shame. How did it break? I will look into it as soon as possible. |
|
|
||
| for i in 1 ..< 5: | ||
| if i < params.len: | ||
| result[i] = params[i].parseInt().clong |
There was a problem hiding this comment.
Ok, I think I found an error. Theses indices are wrong. The ith-param should go into the i+1 position of the result because the position 0 is for the atom. Sorry about this.
A correct version would be something like
for i in 0 ..< params.len:
if i + 1 < result.len:
result[i + 1] = params[i].parseInt().clongThere was a problem hiding this comment.
Or something like this to have the same looping order.
for i in 1 ..< result.len:
if i - 1 < params.len:
result[i] = params[i-1].parseInt().clong
|
Oh yeah, that was definitely the problem, the data being sent to the server didn't really make any sense. Thanks for looking into it, can you make a new pr and replace wormc.nim with the fixed refactored version? |
|
Sure. |
2 participants
Hi, just found about this project and I really like it 💯. I was skimming through the code and noticed several simple things that could be improved. No new functionality, just some cleaner code.
Feel free to nitpick stylistic choices. Thanks for the awesome project!