[FIX] UI Bitrot

[Timmy Douglas]

Todd Fleming <todd at flemingcnc.com> writes:

> I'm really impressed with Timmy's reverse-engineering work.  He came
> really close to finding Bug 2 as evidenced by the following quotes:
>
>> Since "constructors:" is plural, I added the curly braces
>> there but it seems weird that the list would always have only one
>> element in it.
>
> and
>
>> The whole right column of (osWindow blah) is unused.
>
>
> This patch fixes several UI bugs; it runs again.
>
> Note: Do *not* apply to patch that Timmy posted today; it is based on
> an unfortunate misunderstanding caused by Bug 2.
>
> Bug 1
> =====
> src/ui/graphics.slate is in AutoLoad, which broke the instructions
> in src/ui/README.  "load: 'src/ui/graphics.slate'" caused the file to
> load twice.  This is why #parents: misbehaved and the call to #text:
> failed.

wait, you lost me there. how would loading it twice break something?

> Bug 2
> =====
> Fortunately I keep old sources around to compare with.  Unfortunately
> someone made wholesale cosmetic changes to
> plugins/sdl-windows/windows.slate which made it difficult to compare.
> One of those cosmetic changes introduced a bug.
>
> Someone changed:
>   Windows eventTable add:
>       (each key) ->
>       (Windows EventTableEntry
>           newPrototype: (events atSlotNamed: (each value at: 0))
>           constructors: (each value at: 1))].
>
> To:
>   Windows eventTable at: each key put:
>     (Windows EventTableEntry
>        newPrototype: (events atSlotNamed: each value first)
>        constructors: each value first)].
>
> This should be:
>   Windows eventTable at: each key put:
>     (Windows EventTableEntry
>        newPrototype: (events atSlotNamed: each value first)
>        constructors: each value second)].
>
> This caused constructors to be initialized with a (wrong) symbol
> instead of a list.

yeah, I found that and it was in the patch I posted. I think I
remember looking at the old code but I never noticed it then since I
was probably thinking in smalltalk's one-indexed mode. tricky...



> Bug 3
> =====
> Added "lobby" in several places.

what is wrong about this? I think adding slots needs to have an
explicitly-specified namespace. I didn't know what namespace to use
besides "lobby" though.


>> Timmy: It looks like prototype is just the name of the class to copy
>> for the event and constructors are a list of symbols that are
>> applied after it is copied through the 'construct:' message.
>
> Almost correct; prototype isn't just a name of a something to copy;
> it's an actual prototype.
> "newPrototype: (events atSlotNamed: each value first)" grabs the
> prototype from the events namespace.

thanks

>> Timmy: Also it looks like there is a case issue. They use
>> #(MouseLeaveEvent ...) up there but then in the methods there is:
>>
>> event@(Event traits) construct: _@#mouseMotionPosition [...]
>>
>> I can't find a _ at mouseLeaveEvent or _ at MouseLeaveEvent though.
>
> #MouseLeaveEvent refers to a prototype in the events namespace.  The
> right column contains the symbols used for "event@(Event traits)
> construct: _@#blah".  They aren't capitalized because they are not
> prototypes.

ok. I wrote a little something about that in the patch message I sent
earlier. I guess I think it could be a little misleading with the
names matching up like that but with the subtle case difference. Maybe
something like #fetchMousePosition would have made it so I caught on
earlier.