Re: Improvement to games/yquake2

I iterated over your diff like we discussed on #openbsd-gaming.

Added dedicated 1 (good suggestion) and introduced daemon_flags.
Those two changes combined with the +gamedir flag you discovered
allows us to drop rc_start completely.

I'm currently looking for an OK to commit this :)

Regards,
Adam

On Wed, Jun 24, 2020 at 11:12:14PM +0200, Adam Wolk wrote:
> Hi,
>
> On Wed, Jun 24, 2020, at 10:57 AM, Tom Murphy wrote:
> > Hi,
> >
> > Running q2ded in a tty/tmux window, and then if that tty disappears,
> > q2ded begins spinning the CPU taking up sometimes 97% of the CPU.
> > The process is constantly polling for input, but if the tty is
> > marked 'bad' in fstat, the program can't cope.
> >
> > Setting 'busywait' variable to 1 allows q2ded to use nanosleep and
> > stop spinning the cpu.
> >
>
> Great find.
>
> > I've improved the rc script to include the busywait variable and
> > also removed the cd into the shared yquake2 directory since setting
> > +set basedir to that path does the same thing and is cleaner.
> >
> > I've used the following flags for testing:
> >
> > q2ded_flags="+set dedicated 1 +set game holywars +exec server.cfg"
> >
> > It might be worth also hard coding +set dedicated 1 as well in the
> > rc script? What do you think?
> >
>
> I think it makes sense for the rc.d script. If someone wants to run it in the foreground
> he/she can cd to the game directory manually and execute the binary directly. That
> is the reason we documented in the commit msg for not providing a q2ded wrapper
> like we do for the game executable itself.
>
> One additional comment inline the diff.
>
> > Diff is attached.
> >
> > Thanks,
> > Tom
> >
> >
> > Index: Makefile
> > - ${rcexec} "cd ${TRUEPREFIX}/share/yquake2; ${daemon} ${daemon_flags}"
> > + ${rcexec} "${daemon} +set gamedir ${TRUEPREFIX}/share/yquake2 +set
> > busywait 0 ${daemon_flags}"
>
> I think I don't like having this explicitly added to the command here. That way no one
> can override those flags even for testing. I think a cleaner solution would be to define
> daemon_flags and have those as default like we do for ie. postgresql. That way people
> who want to override them just define their own in rc.conf.local and I think it's reasonable
> to expect them to understand why they should copy the existing ones. I think this
> would also allow us to remove the whole rc_start definition.
>
>