[Ohrrpgce] SVN: james/5080 Fix broken slice mutators. slices.bas compiles again now

[Ralph Versteegen]

On 29 February 2012 07:07, James Paige <Bob at hamsterrepublic.com> wrote:
> On Tue, Feb 28, 2012 at 09:53:59AM -0800, Jay Tennant wrote:
>> > From: James Paige <Bob at HamsterRepublic.com>
>> > Sent: Tuesday, February 28, 2012 11:13 AM
>> >
>> > On Tue, Feb 28, 2012 at 09:04:21AM -0800, subversion at HamsterRepublic.com wrote:
>> > > james
>> > > 2012-02-28 09:04:21 -0800 (Tue, 28 Feb 2012)
>> > > 57
>> > > Fix broken slice mutators. slices.bas compiles again now
>> > > ---
>> > > U   wip/slices.bas
>> >
>> > I have several thoughts about these accessors and mutators.
>> >
>> > the OHR code is riddled with crazy naming schemes and inconsistent
>> > style, but slices.bas is wone of the few files that is well organized
>> > and consistently style, so it would be nice to keep it that way.
>> >
>> > What I notice especially is that all the existing slice related
>> > functions all take the Slice ptr as the first argument. I would like it
>> > if these new slice mutator functions would do the same.
>>
>> That sounds fine. I can change that.
>
> Cool :)
>
>> > There are other style issues like spaces instead of tabs, or names in
>> > the form of SomethingSomethingSlice() instead of
>> > sliceSomethingSomething() but those are of much less importance to me
>> > than the argument order.
>>
>> I can use spaces instead of tabs. I'll change that as well. The naming
>> kind of interests me. I notice there's a SliceDraw as well as DrawSlice,
>> and some of the functions begin with Slice, and others do not (ie.
>> SliceLoadFromFile, SliceDebugRemember, SliceXAnchor, SliceCollidePoint,
>> etc.). In my personal experience, I find it easier to understand context
>> when dealing with objects in object-oriented programming than with
>> procedural programming because the functions are grouped with the
>> object's class (or instance, if prototype-based language). Since our
>> use is between multiple languages, I like prefixing an ID to the
>> function name to relate it to the objects on which it operates. I'm sure
>> you can think of libraries that follow this practice.
>
> Yeah, the current naming scheme is a bit haphazard, and SliceSomething
> does actually seem a little better than SomethingSlice.
>
>> I can use the SomethingSomethingSlice() naming convention instead if
>> it is preferred. I agree that consistency would be golden, so I'll work
>> towards that end. With the large number of functions present in the
>> engine, the prefixed ID does act as a kind of convenient namespace.
>
> Looking over the names right now, functions that do something to the
> slice are mostly form of VerbSlice() which seems logical to me. accessor functions that read
> something from the slice are in the form of SliceNoun() where Noun is
> the name of the thing that is being read. A few are in the form of
> VerbSliceNoun() for functions that do something to a slices data. A few
> of them don't have "Slice" in the name at all.
>
> I guess if we really wanted to clean things up, we would use an actual
> namespace like the one in reload.bas and leave "Slice" out of ALL the
> function names. Then other code would call slices.Verb() or
> slices.getNoun() and slices.setNoun() ... however, changing them all now
> seems a like a ton of work for a rather small payoff.
>
> I am actually fine with either SliceSomethingSomething or
> SomethingSomethingSlice, so I can live with either one, or a mixture.

The mixture is fine; it's logical. SliceX makes a lot more sense than XSlice.

>> > I was also thinking about null ptr checking. Most of the other slice
>> > functions do explicit null ptr checking. In the past, this was a really
>> > big deal for me because null ptr bugs used to be INSANELY frustrating to
>> > debug. However, now that we can run in gdb, and use valgrind (and now
>> > that I know how to do both of those things) I guess I don't have to be
>> > the same stickler for null ptr checking that I used to be...
>>
>> Null ptr checking sounds good. You can modify the code any way you see
>> fit. I don't have feelings attached to this code. ;)
>
> Yes, I could easily add the null checks. I am just wondering if it
> actually is better to skip the null ptr checking and rely on gdb and
> valgrind. I am feeling indecisive about this subject. I would value
> TMC's input on this, because he as a more skilled user of both those
> tools than I am.
>
> ---
> James

I would lean towards having only one way to do it: access Slice
members directly everywhere instead of having to decide whether or not
to use the accessors, and provide the accessors only for non-FB code.

We already have lots of null-pointer checking everywhere, such as in
valid_plotslice. If an unexpected null pointer did creep in somehow,
then I think it's better to crash violently than silently ignore the
bug and never find out about it.

In addition, I think there's a much bigger chance of having trouble
with stale pointers (eg. something in the GameSlices table being
deleted when it shouldn't have been), and you can't check for those.


It might be worth having (non-trivial) ScreenX/Y accessors though, to
hide the updating of screen position. If only there was some way to
update the screen position only when needed rather than every call...