#openttdcoop

(12:15:59) Topic for #freerct set by ChanServ!services@services.oftc.net at 08:48:33 on 2014-03-05
(12:15:59) mode (+o Alberth) by ChanServ
(12:16:05) LordAro: hihi
(12:16:13) Alberth: moin
(12:16:16) LordAro: i was just wondering when you were going to turn up :)
(12:16:30) Alberth: I had to install fedora 21 :)
(12:16:37) LordAro: :o
(12:16:54) LordAro: needs more Arch :p
(12:17:41) Alberth: Argh?  :p
(12:17:47) LordAro: :p
(12:30:46) Alberth: I very much wonder what to do next
(12:32:33) LordAro: i've made progress on the xyz stuff
(12:32:37) LordAro: still lots to go though
(12:50:55) Alberth: it makes 3d position stuff a bit useless to work on
(12:52:52) Alberth: the question is what to do instead
(13:02:33) LordAro: :3
(13:02:41) LordAro: ooh, i found a bug
(13:02:45) LordAro: if (vstack != nullptr) vp->MarkVoxelDirty(pt.x, pt.x, vstack->base, vstack->height);
(13:04:39) Alberth: ...
(13:05:21) Alberth: oh 
(13:05:26) Alberth: :p
(13:09:06) LordAro: seems it got converted at some point and not noticed either :p
(13:18:29) LordAro: hmm
(13:18:57) LordAro: my (Point<CT>, CT z) as caused a lot of issues :3
(13:19:44) Alberth: yeah, I refrained from converting anything that is not a true 3d position
(13:20:53) Alberth: a  3d -> 2d  (for x,y)   may be useful, as is a     3d, delta_z    it seems
(13:21:07) Alberth: but just 3d points is already a lot of work
(13:21:49) LordAro: mm
(13:22:17) LordAro: i've added a + operator for the (usual) delta_z type things
(13:22:36) LordAro: although there are some other cases where all 3 coords are added to
(13:26:58) Alberth: +operator is too confusing for that imho
(13:27:32) Alberth: + on a vector should add a vector rather than a single number
(13:28:03) Alberth: XYZPoint XYZPoint::AddZ(int delta_z)   could be useful
(13:28:44) Alberth: hmm, is "Add" the correct word for that?
(13:36:27) LordAro: the + operator is adding a verctor
(13:37:05) LordAro: e.g. fdata.voxel_pos + XYZPoint16(0, 0, extra_z)
(13:37:15) Alberth: ok, that's fine
(13:37:23) Alberth: and a nice solution, imho
(13:38:02) Alberth: didn't think of that :)
(13:38:02) ParkWizard: This park is really clean and tidy.
(13:38:17) Alberth: duh, we don't have litter yet :p
(13:41:42) LordAro: :)
(15:23:41) Henke37 [~Henrik@2001:2002:4e48:8217:f0bf:8440:b5d6:ccf5] entered the room.
(16:04:03) sgtbigman [~sgtbigman@104-48-204-167.lightspeed.bcvloh.sbcglobal.net] entered the room.
(16:04:17) sgtbigman: o7
(16:04:39) LordAro: o7
(16:05:43) sgtbigman: PersonGraphics updates - https://paste.openttdcoop.org/pquzamwvs
(16:05:49) Alberth: hihi
(16:06:16) Alberth: doesn't exist?
(16:06:33) sgtbigman: oh oops :)
(16:08:09) sgtbigman: https://paste.openttdcoop.org/prdtzhh24
(16:08:09) ParkWizard: #openttdcoop - Paste
(16:09:18) Alberth: should it also document version 1?
(16:09:31) Alberth: not sure about this
(16:21:43) Alberth: for block fields, there are start and end versions, these numbers in lists are not versioned that way
(16:23:16) Alberth: for (PersonType pertype = PERSON_GUEST; pertype <= PERSON_GUEST; pertype++) {   <-- bit silly to do for 1 value :p
(16:24:04) LordAro: really need to find a way to make the person stuff smaller :)
(16:25:05) Alberth: if (ptype < PERSON_GUEST || ptype > PERSON_GUEST) return false;     <-- very complicated way of !=     line below can also be simplified
(16:26:56) Alberth: +   if (rcd_file->version < 1 || length < 1) return false;   <-- you don't want to accept all future versions, do you?
(16:28:46) Alberth: +           case 16:
(16:28:46) Alberth: +               pt = PERSON_GUEST;
(16:28:46) Alberth: +               break;
(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
(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)
(16:32:18) sgtbigman: thanks for the help
(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
(16:32:54) sgtbigman: rcd_file->version < 1 || rcd_file->version > 2 return false; ?
(16:33:12) Alberth: sure
(16:33:21) sgtbigman: the extrace space may have slipped through...
(16:33:32) sgtbigman: right-align, ok, sounds good :)
(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 :)
(16:34:24) Alberth: did you read it yourself afterwards?
(16:34:56) Alberth: I always do that before committing, to see if something weird slipped in
(16:34:59) sgtbigman: probably briefly the other day when I made it :)
(16:35:08) sgtbigman: definitely a good idea
(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
(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
(16:37:35) Alberth: with text documents, I often wait 1 or 2 days before reading, as I can freshly read it again then
(16:38:32) Alberth: +   {"entertainer",  20},
(16:38:32) Alberth:       {"grey",         COL_GREY},   <-- here your double space originates from, I suspect :)
(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
(16:40:34) Alberth: (not sure whether that is the case here)
(16:41:18) Alberth: DecodePersonType   is also not consistent with case layout
(16:41:31) Alberth: and the final hunk is a bug fix, ie a separate patch
(16:42:19) Alberth: Hmm, I used pre-increment on iter2?    How weird :p
(16:42:30) Alberth: I usually use post-increment
(16:43:09) sgtbigman: :p
(16:43:38) Alberth: +   PERSON_MIN_STAFF = PERSON_HANDYMAN,    ///< First value of an employee.
(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?
(16:43:51) sgtbigman: ok, so vertically-aligning the guest through entertainer part is probably ok, but not with the entire table's contents :)
(16:43:52) Alberth: if not, delete them
(16:44:32) sgtbigman: I believe you had the MIN_GUEST and MAX_GUEST for loading valid person types
(16:44:43) sgtbigman: so I guess eventually, the Staff code could use those
(16:44:52) sgtbigman: but not positive if they would be necessary
(16:44:57) Alberth: this case of vertical aligning is probably fine either way
(16:45:14) Alberth: there is a grey area inbetween
(16:45:58) Alberth: just remove them from the patch, you can always add it later
(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
(16:47:24) Alberth: there is only 1 guest type, so testing it is useless
(16:47:34) Alberth: different patch to remove that though, imho
(16:48:11) ***Alberth wonders where valid_ptypes comes from
(16:49:50) Alberth: oh, from loading?
(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
(16:52:06) Alberth: just mentioning it, to ensure you are aware of this hole
(16:53:40) Alberth: -       if (usable) this->valid_ptypes |= 1u << (pertype - PERSON_MIN_GUEST);
(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)
(16:55:17) sgtbigman: PERSON_MIN_GUEST pointed to PERSON_ANY
(16:55:50) Alberth: ah!  we had 3 guest types thus! :)
(16:55:57) Alberth: that was a bug :p
(16:56:32) Alberth: well, assuming you're removing valid_ptypes anyway, just leave it
(16:57:10) Alberth: Even just adding a note "@todo valid_ptypes should be removed" would be ok
(16:57:57) Alberth: as you change it means it still all works  even if it serves no real purpose :)
(17:00:14) sgtbigma1 [~sgtbigman@104-48-204-167.lightspeed.bcvloh.sbcglobal.net] entered the room.
(17:01:17) sgtbigman left the room (quit: Read error: Connection reset by peer).
(17:02:08) sgtbigma1 left the room (quit: ).
(17:02:23) sgtbigman [~sgtbigman@104-48-204-167.lightspeed.bcvloh.sbcglobal.net] entered the room.
(17:02:36) sgtbigman: laptop freezing on me :/
(17:03:36) Alberth: cpus like cold laptops :p
(17:11:00) sgtbigman: yep :p
(17:35:14) sgtbigman: for the switch case format
(17:35:21) sgtbigman: case 8:
(17:35:33) sgtbigman: case 16: return PERSON_GUEST;
(17:36:06) sgtbigman: since the rest have that format
(17:36:09) Henke37: I prefer type objects instead of type ids
(17:57:08) Alberth: seems fine sgtbigman
(17:57:16) Alberth: Henke37:  use Java
(17:57:36) Henke37: that was a random suggestion
(17:57:53) sgtbigman: if (usable) this->valid_ptypes |= 1u << (pertype - PERSON_ANY);
(17:57:56) Henke37: how do you reason to go from my suggestion to your?
(17:58:06) sgtbigman: not valid any longer Alberth?
(17:58:57) Alberth: it is valid until you delete valid_ptypes, isn't it?
(17:59:17) Alberth: Henke37: Java has enum value objects
(17:59:38) Alberth: oh, and class/type objects too :)
(18:00:35) sgtbigman: So you are suggesting that valid_ptypes is no longer needed?
(18:00:56) sgtbigman: apologies, trying to remember everything stated earlier
(18:01:22) sgtbigman: when I lost connection, I unfortunately lost the chat history...but I remember most of it ;)

Mark as private