Loading

Paste #pz3pqnkdj

Wrap Raw Revise
  1. (12:15:59) Topic for #freerct set by ChanServ!services@services.oftc.net at 08:48:33 on 2014-03-05
  2. (12:15:59) mode (+o Alberth) by ChanServ
  3. (12:16:05) LordAro: hihi
  4. (12:16:13) Alberth: moin
  5. (12:16:16) LordAro: i was just wondering when you were going to turn up :)
  6. (12:16:30) Alberth: I had to install fedora 21 :)
  7. (12:16:37) LordAro: :o
  8. (12:16:54) LordAro: needs more Arch :p
  9. (12:17:41) Alberth: Argh?  :p
  10. (12:17:47) LordAro: :p
  11. (12:30:46) Alberth: I very much wonder what to do next
  12. (12:32:33) LordAro: i've made progress on the xyz stuff
  13. (12:32:37) LordAro: still lots to go though
  14. (12:50:55) Alberth: it makes 3d position stuff a bit useless to work on
  15. (12:52:52) Alberth: the question is what to do instead
  16. (13:02:33) LordAro: :3
  17. (13:02:41) LordAro: ooh, i found a bug
  18. (13:02:45) LordAro: if (vstack != nullptr) vp->MarkVoxelDirty(pt.x, pt.x, vstack->base, vstack->height);
  19. (13:04:39) Alberth: ...
  20. (13:05:21) Alberth: oh
  21. (13:05:26) Alberth: :p
  22. (13:09:06) LordAro: seems it got converted at some point and not noticed either :p
  23. (13:18:29) LordAro: hmm
  24. (13:18:57) LordAro: my (Point<CT>, CT z) as caused a lot of issues :3
  25. (13:19:44) Alberth: yeah, I refrained from converting anything that is not a true 3d position
  26. (13:20:53) Alberth: a  3d -> 2d  (for x,y)   may be useful, as is a     3d, delta_z    it seems
  27. (13:21:07) Alberth: but just 3d points is already a lot of work
  28. (13:21:49) LordAro: mm
  29. (13:22:17) LordAro: i've added a + operator for the (usual) delta_z type things
  30. (13:22:36) LordAro: although there are some other cases where all 3 coords are added to
  31. (13:26:58) Alberth: +operator is too confusing for that imho
  32. (13:27:32) Alberth: + on a vector should add a vector rather than a single number
  33. (13:28:03) Alberth: XYZPoint XYZPoint::AddZ(int delta_z)   could be useful
  34. (13:28:44) Alberth: hmm, is "Add" the correct word for that?
  35. (13:36:27) LordAro: the + operator is adding a verctor
  36. (13:37:05) LordAro: e.g. fdata.voxel_pos + XYZPoint16(0, 0, extra_z)
  37. (13:37:15) Alberth: ok, that's fine
  38. (13:37:23) Alberth: and a nice solution, imho
  39. (13:38:02) Alberth: didn't think of that :)
  40. (13:38:02) ParkWizard: This park is really clean and tidy.
  41. (13:38:17) Alberth: duh, we don't have litter yet :p
  42. (13:41:42) LordAro: :)
  43. (15:23:41) Henke37 [~Henrik@2001:2002:4e48:8217:f0bf:8440:b5d6:ccf5] entered the room.
  44. (16:04:03) sgtbigman [~sgtbigman@104-48-204-167.lightspeed.bcvloh.sbcglobal.net] entered the room.
  45. (16:04:17) sgtbigman: o7
  46. (16:04:39) LordAro: o7
  47. (16:05:43) sgtbigman: PersonGraphics updates - https://paste.openttdcoop.org/pquzamwvs
  48. (16:05:49) Alberth: hihi
  49. (16:06:16) Alberth: doesn't exist?
  50. (16:06:33) sgtbigman: oh oops :)
  51. (16:08:09) sgtbigman: https://paste.openttdcoop.org/prdtzhh24
  52. (16:08:09) ParkWizard: #openttdcoop - Paste
  53. (16:09:18) Alberth: should it also document version 1?
  54. (16:09:31) Alberth: not sure about this
  55. (16:21:43) Alberth: for block fields, there are start and end versions, these numbers in lists are not versioned that way
  56. (16:23:16) Alberth: for (PersonType pertype = PERSON_GUEST; pertype <= PERSON_GUEST; pertype++) {   <-- bit silly to do for 1 value :p
  57. (16:24:04) LordAro: really need to find a way to make the person stuff smaller :)
  58. (16:25:05) Alberth: if (ptype < PERSON_GUEST || ptype > PERSON_GUEST) return false;     <-- very complicated way of !=     line below can also be simplified
  59. (16:26:56) Alberth: +   if (rcd_file->version < 1 || length < 1) return false;   <-- you don't want to accept all future versions, do you?
  60. (16:28:46) Alberth: +           case 16:
  61. (16:28:46) Alberth: +               pt = PERSON_GUEST;
  62. (16:28:46) Alberth: +               break;
  63. (16:28:46) Alberth: +           case 17: pt = PERSON_HANDYMAN; break;    <-- be consistent in code layout, either the previous one or this one (and all below) should be changed
  64. (16:31:08) Alberth: enum PersonType  <-- drop the explicit numbers there (they are omitted normally, the old ones probably exist because a first version used gaps in the numbering)
  65. (16:32:18) sgtbigman: thanks for the help
  66. (16:32:47) Alberth: +   {"entertainer",  20},   <-- why more than 1 space in ",  2"  ?  also, please right-align numbers, as it's easier to read them
  67. (16:32:54) sgtbigman: rcd_file->version < 1 || rcd_file->version > 2 return false; ?
  68. (16:33:12) Alberth: sure
  69. (16:33:21) sgtbigman: the extrace space may have slipped through...
  70. (16:33:32) sgtbigman: right-align, ok, sounds good :)
  71. (16:34:10) Alberth: oh, np, I know from experience it's amazing how much simple stuff slips through if you give someone else a patch :)
  72. (16:34:24) Alberth: did you read it yourself afterwards?
  73. (16:34:56) Alberth: I always do that before committing, to see if something weird slipped in
  74. (16:34:59) sgtbigman: probably briefly the other day when I made it :)
  75. (16:35:08) sgtbigman: definitely a good idea
  76. (16:35:42) sgtbigman: yes, if this was a commit, I'd be re-reading a lot...tends to take me twice as long to commit as it should :p
  77. (16:37:00) Alberth: it takes practice, but in time you'll note what you often mess up, and watch out for it more carefully
  78. (16:37:35) Alberth: with text documents, I often wait 1 or 2 days before reading, as I can freshly read it again then
  79. (16:38:32) Alberth: +   {"entertainer",  20},
  80. (16:38:32) Alberth:       {"grey",         COL_GREY},   <-- here your double space originates from, I suspect :)
  81. (16:40:22) Alberth: It's nice to have vertically aligned stuff, but if you do it for entire long tables with different content, you'll find that you need large amounts of white space. At that point you may want to vertically align parts independently
  82. (16:40:34) Alberth: (not sure whether that is the case here)
  83. (16:41:18) Alberth: DecodePersonType   is also not consistent with case layout
  84. (16:41:31) Alberth: and the final hunk is a bug fix, ie a separate patch
  85. (16:42:19) Alberth: Hmm, I used pre-increment on iter2?    How weird :p
  86. (16:42:30) Alberth: I usually use post-increment
  87. (16:43:09) sgtbigman: :p
  88. (16:43:38) Alberth: +   PERSON_MIN_STAFF = PERSON_HANDYMAN,    ///< First value of an employee.
  89. (16:43:38) Alberth: +   PERSON_MAX_STAFF = PERSON_ENTERTAINER, ///< Last value of an employee.  <-- these are not used here, do you intend to use them?
  90. (16:43:51) sgtbigman: ok, so vertically-aligning the guest through entertainer part is probably ok, but not with the entire table's contents :)
  91. (16:43:52) Alberth: if not, delete them
  92. (16:44:32) sgtbigman: I believe you had the MIN_GUEST and MAX_GUEST for loading valid person types
  93. (16:44:43) sgtbigman: so I guess eventually, the Staff code could use those
  94. (16:44:52) sgtbigman: but not positive if they would be necessary
  95. (16:44:57) Alberth: this case of vertical aligning is probably fine either way
  96. (16:45:14) Alberth: there is a grey area inbetween
  97. (16:45:58) Alberth: just remove them from the patch, you can always add it later
  98. (16:47:06) Alberth: +assert_compile(PERSON_GUEST + 1 <= 16); ///< Verify that all person types fit in #Guests::valid_ptypes     this is weird now too, I think
  99. (16:47:24) Alberth: there is only 1 guest type, so testing it is useless
  100. (16:47:34) Alberth: different patch to remove that though, imho
  101. (16:48:11) ***Alberth wonders where valid_ptypes comes from
  102. (16:49:50) Alberth: oh, from loading?
  103. (16:51:32) Alberth: technically, there is the problem that you accept eg 17 from a version 1 block. I don't have much problems with that tbh
  104. (16:52:06) Alberth: just mentioning it, to ensure you are aware of this hole
  105. (16:53:40) Alberth: -       if (usable) this->valid_ptypes |= 1u << (pertype - PERSON_MIN_GUEST);
  106. (16:53:40) Alberth: +       if (usable) this->valid_ptypes |= 1u << (pertype - PERSON_ANY);   <-- here you change the meaning of bit 0 (and also all other bits as a consequence)
  107. (16:55:17) sgtbigman: PERSON_MIN_GUEST pointed to PERSON_ANY
  108. (16:55:50) Alberth: ah!  we had 3 guest types thus! :)
  109. (16:55:57) Alberth: that was a bug :p
  110. (16:56:32) Alberth: well, assuming you're removing valid_ptypes anyway, just leave it
  111. (16:57:10) Alberth: Even just adding a note "@todo valid_ptypes should be removed" would be ok
  112. (16:57:57) Alberth: as you change it means it still all works  even if it serves no real purpose :)
  113. (17:00:14) sgtbigma1 [~sgtbigman@104-48-204-167.lightspeed.bcvloh.sbcglobal.net] entered the room.
  114. (17:01:17) sgtbigman left the room (quit: Read error: Connection reset by peer).
  115. (17:02:08) sgtbigma1 left the room (quit: ).
  116. (17:02:23) sgtbigman [~sgtbigman@104-48-204-167.lightspeed.bcvloh.sbcglobal.net] entered the room.
  117. (17:02:36) sgtbigman: laptop freezing on me :/
  118. (17:03:36) Alberth: cpus like cold laptops :p
  119. (17:11:00) sgtbigman: yep :p
  120. (17:35:14) sgtbigman: for the switch case format
  121. (17:35:21) sgtbigman: case 8:
  122. (17:35:33) sgtbigman: case 16: return PERSON_GUEST;
  123. (17:36:06) sgtbigman: since the rest have that format
  124. (17:36:09) Henke37: I prefer type objects instead of type ids
  125. (17:57:08) Alberth: seems fine sgtbigman
  126. (17:57:16) Alberth: Henke37:  use Java
  127. (17:57:36) Henke37: that was a random suggestion
  128. (17:57:53) sgtbigman: if (usable) this->valid_ptypes |= 1u << (pertype - PERSON_ANY);
  129. (17:57:56) Henke37: how do you reason to go from my suggestion to your?
  130. (17:58:06) sgtbigman: not valid any longer Alberth?
  131. (17:58:57) Alberth: it is valid until you delete valid_ptypes, isn't it?
  132. (17:59:17) Alberth: Henke37: Java has enum value objects
  133. (17:59:38) Alberth: oh, and class/type objects too :)
  134. (18:00:35) sgtbigman: So you are suggesting that valid_ptypes is no longer needed?
  135. (18:00:56) sgtbigman: apologies, trying to remember everything stated earlier
  136. (18:01:22) sgtbigman: when I lost connection, I unfortunately lost the chat history...but I remember most of it ;)
Posted by Anonymous at 01 Jan 2015, 17:03:53 UTC
Language: text • Views: 622

Comments