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.
>
>
? q2ded_flags.diff
Index: Makefile
===================================================================
RCS file: /cvs/ports/games/yquake2/Makefile,v
retrieving revision 1.21
diff -u -p -r1.21 Makefile
--- Makefile 25 Mar 2020 21:22:59 -0000 1.21
+++ Makefile 27 Jun 2020 13:17:46 -0000
@@ -8,7 +8,7 @@ V= 7.43
PKGNAME= ${N}-${V}
DISTNAME= quake2-${V}
CATEGORIES= games
-REVISION= 0
+REVISION= 1
HOMEPAGE= https://www.yamagi.org/quake2/
MAINTAINER= Adam Wolk <[email protected]>
Index: pkg/q2ded.rc
===================================================================
RCS file: /cvs/ports/games/yquake2/pkg/q2ded.rc,v
retrieving revision 1.5
diff -u -p -r1.5 q2ded.rc
--- pkg/q2ded.rc 11 Jan 2018 19:27:02 -0000 1.5
+++ pkg/q2ded.rc 27 Jun 2020 13:17:46 -0000
@@ -3,6 +3,7 @@
# $OpenBSD: q2ded.rc,v 1.5 2018/01/11 19:27:02 rpe Exp $
daemon="${TRUEPREFIX}/share/yquake2/q2ded"
+daemon_flags="+set busywait 0 +set dedicated 1 +set gamedir
${TRUEPREFIX}/share/yquake2"
daemon_user="_yquake2"
. /etc/rc.d/rc.subr
@@ -10,9 +11,5 @@ daemon_user="_yquake2"
pexp="${daemon}.*"
rc_bg=YES
rc_reload=NO
-
-rc_start() {
- ${rcexec} "cd ${TRUEPREFIX}/share/yquake2; ${daemon} ${daemon_flags}"
-}
rc_cmd $1
