On 2/14/22 10:57, jr wrote:
> hi,
> 
> William F Pokorny<ano### [at] anonymousorg>  wrote:
>> Feedback? Thoughts?
> only a couple of "things".  (sorry took a few days)

No worries.

> 
> I (still) think that the .inc would be a good place to store other
> 'povr'-specific stuff too, like the ids.
> 

Here, I wonder if perhaps the scale of change in the povr branch is 
being seen as less extensive than it in fact is. The new setidtypes.inc 
is one of several completely new include files.

Nearly all the remaining, shipped include files are substantially 
different. Only rand.inc hasn't been changed much - mostly because I see 
a good many puzzles there in code that I don't think many use. I'm not 
sure what to do povr's current rand.inc as yet.

The povr branch only ships 14 includes - to be 15 with forkversionhook.inc.

Lastly, an aim is that the 'forkversionhook.inc' file be extensible. 
That, in approach, its able to accommodate multiple forks / patches / 
branches. In other words, in just supporting multiple forks over time, 
it could grow substantially in size. I see no reason this file cannot be 
shared - at least to some degree - across forks.

> from the looks of 'forkversionhook.inc' it will be only for 'povr', I'd have
> hoped for a 'version.inc' which would work regardless, for any version and/or
> fork of POV-Ray.

I'll try to reduce / rework the mentions of povr.

The current forkversionhook.inc does work for any POV-Ray version - at 
least when I've tested it.

It's only povr specific when the SDL variable Dump_povr_fork_ is set on 
inclusion. Then it prints documentation as to what povr fork_*() 
constants are available and stops - and it only does that much if the 
fork being run is in fact 'povr'. I'll add a commented #debug where I 
had an empty else block indicating something which could be printed when
the fork hook is not found. It's commented because I don't want to 
default to always printing something on inclusion of
forkversionhook.inc. Regular include use will be to make the 
'Have_f_odd_fork_hook()' macro available to scene code and nothing more.

> Have_f_odd_fork_hook()
> lastly, the constant + macro names are, um, a little verbose?:-)
> 

Agree.

In general, I think I'll reduce the f_odd() visibility. No reason day to 
day users need to know this the internal 'hook'.

I also think you're right it's better to call the fork_*()s, constants, 
rather than functions.

Beyond that I'll take you to mean, perhaps, something like the following:

forkversionhook.inc -->  forkhooks.inc ?

Have_f_odd_fork_hook() --> HaveForkConstants() ?

Dump_povr_fork_  --> Fork_povr ?

...I'm less a fan of shortening the constant pairs, but...

fork_str(n) --> frkstr(n) ?
fork_val(n) --> frkval(n) ?

Better?

Bill P.

Post a reply to this message