MantisBT - Zandronum
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0001236Zandronum[All Projects] Suggestionpublic2012-12-30 19:232018-09-30 20:19
ReporterKonar6 
Assigned ToDusk 
PrioritynoneSeverityfeatureReproducibilityN/A
StatusclosedResolutionfixed 
Platform*OSOS Version
Product Version1.0 
Target Version1.1Fixed in Version1.1 
Summary0001236: Suggestions for client console notify lines
Description1]
The number of console notify lines is hardcoded to 4 in c_console.cpp:204.
I imagine that it originates from the old Doom days when it was sufficient for the few players Doom had supported + monitor resolutions were low, but today we have 64 players support and a lot more stuff is printed in the console, and we have monitors with huge resolutions.
Personally I've doubled the number of notify lines, and I thought it would be a blast if this value was user-configurable via some con_notifylines CVAR, just like there is con_notifytime for adjusting the time for the messages to stay on screen.
I would have provided a patch, but I don't know how I would go about changing that constant to CVAR variable since it's used in arrays :(.
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
related to 0001377feedback  Suggestions for client console notify lines, part 2 
Attached Filespng Screenshot_Doom_20121230_231654.png (141,915) 2012-12-30 23:00
/tracker/file_download.php?file_id=907&type=bug
png
Issue History
Date ModifiedUsernameFieldChange
2012-12-30 19:23Konar6New Issue
2012-12-30 21:38Torr SamahoNote Added: 0005626
2012-12-30 22:25DuskNote Added: 0005627
2012-12-30 22:25DuskAssigned To => Dusk
2012-12-30 22:25DuskStatusnew => needs review
2012-12-30 22:29DuskNote Edited: 0005627bug_revision_view_page.php?bugnote_id=5627#r3087
2012-12-30 22:49Konar6Note Added: 0005628
2012-12-30 22:56DuskNote Edited: 0005627bug_revision_view_page.php?bugnote_id=5627#r3088
2012-12-30 22:56DuskNote Added: 0005629
2012-12-30 23:00Konar6File Added: Screenshot_Doom_20121230_231654.png
2012-12-30 23:01Konar6Note Edited: 0005628bug_revision_view_page.php?bugnote_id=5628#r3090
2012-12-31 09:50Torr SamahoNote Added: 0005634
2012-12-31 11:04DuskNote Added: 0005639
2012-12-31 11:04DuskStatusneeds review => assigned
2012-12-31 11:14DuskNote Edited: 0005639bug_revision_view_page.php?bugnote_id=5639#r3092
2012-12-31 11:14DuskStatusassigned => needs review
2012-12-31 12:11Torr SamahoNote Added: 0005641
2012-12-31 12:33Torr SamahoStatusneeds review => feedback
2013-01-01 11:07DuskNote Added: 0005646
2013-01-01 11:07DuskStatusfeedback => needs review
2013-01-02 09:13Torr SamahoNote Added: 0005648
2013-01-02 10:41Torr SamahoNote Edited: 0005648bug_revision_view_page.php?bugnote_id=5648#r3100
2013-01-02 10:41Torr SamahoStatusneeds review => needs testing
2013-01-02 13:10ZzZomboNote Added: 0005655
2013-01-09 23:16DuskTarget Version => 1.1
2013-03-20 04:42WatermelonNote Added: 0006149
2013-03-24 13:35Torr SamahoNote Deleted: 0005655
2013-03-24 13:39Torr SamahoNote Added: 0006167
2013-03-28 17:18WatermelonNote Added: 0006180
2013-03-28 17:48DuskNote Added: 0006181
2013-04-08 22:42DuskNote Added: 0006273
2013-04-08 22:42DuskStatusneeds testing => feedback
2013-06-11 01:54QentNote Added: 0006414
2013-06-11 01:55QentNote Edited: 0006414bug_revision_view_page.php?bugnote_id=6414#r3535
2013-06-23 14:48DuskNote Edited: 0005627bug_revision_view_page.php?bugnote_id=5627#r3575
2013-06-23 14:48DuskNote Edited: 0005639bug_revision_view_page.php?bugnote_id=5639#r3576
2013-06-23 14:49DuskRelationship addedrelated to 0001377
2013-06-23 14:49DuskDescription Updatedbug_revision_view_page.php?rev_id=3578#r3578
2013-06-23 14:50DuskNote Added: 0006487
2013-06-23 14:50DuskStatusfeedback => resolved
2013-06-23 14:50DuskFixed in Version => 1.1
2013-06-23 14:50DuskResolutionopen => fixed
2018-09-30 20:19Blzut3Statusresolved => closed

Notes
(0005626)
Torr Samaho   
2012-12-30 21:38   
Quote from Konar6
I would have provided a patch, but I don't know how I would go about changing that constant to CVAR variable since it's used in arrays :(.
One possibility is to replace the fixed size C array with ZDoom's TArray and then to dynamically change the size of the TArray whenever the user changes the proposed CVAR.

Quote from Konar6
Why is C_FlushDisplay() called in g_level.cpp:1022 and g_level.cpp:1579 (ie. on level finished and level start)?

g_level.cpp:1022 was already in the source of ST 97c2, so it was added to Skulltag before I took care of the source, g_level.cpp:1579 is inherited from ZDoom. I don't know the motivation of either.
(0005627)
Dusk   
2012-12-30 22:25   
(edited on: 2013-06-23 14:48)
SURPRISE COMMIT

Maybe con_numnotifies would be a better cvar name?
(0005628)
Konar6   
2012-12-30 22:49   
(edited on: 2012-12-30 23:01)
Very good!

I like con_notifylines since it's consistent with existing con_notifytime.

EDIT: updated a screenshot showing the effect of removing C_FlushDisplay() on level start, which I personally find pretty cool :P.
(0005629)
Dusk   
2012-12-30 22:56   
Renamed, updated commit link
(0005634)
Torr Samaho   
2012-12-31 09:50   
Quote from Dusk
SURPRISE COMMIT

I'd say it's safer to use NotifyStrings.Size() instead of con_numnotifylines.GetGenericRep(CVAR_Int).Int to determine NUMNOTIFIES. If for whatever reason the former should be smaller than the latter, code using the latter to access NotifyStrings will crash. And as always: Please mark any changes to ZDoom code with a comment.
(0005639)
Dusk   
2012-12-31 11:04   
(edited on: 2013-06-23 14:48)
I actually had just thought of the same.. let's see here.

EDIT: How's this?
(0005641)
Torr Samaho   
2012-12-31 12:11   
Looks good! If you rebase and collapse it, I'll pull it, or I can also do the rebasing / collapsing. The only thing I still wonder is whether we should enforce a reasonable maximum for con_notifylines. Setting this in the thousands certainly doesn't make any sense.
(0005646)
Dusk   
2013-01-01 11:07   
Alright, here is this
(0005648)
Torr Samaho   
2013-01-02 09:13   
(edited on: 2013-01-02 10:41)
Thanks! Pulled and rebased. I also prevented con_notifylines from being set to anything bigger than 50. If somebody really wants to set it to something bigger, we can raise the limit.

EDIT: We still need to decide what to do with C_FlushDisplay().
(0006149)
Watermelon   
2013-03-20 04:42   
Is it significant if we enter in a negative number for the time it stays on the screen? When I enter in -1 it holds -1 as a value. I don't know if we want to (like the other cvar) just hold it at 0 if someone goes negative or not.
(0006167)
Torr Samaho   
2013-03-24 13:39   
The way con_notifytime is handled is inherited from ZDoom. We shouldn't change this unless it's changed in ZDoom.
(0006180)
Watermelon   
2013-03-28 17:18   
I'd say then that this is a resolved ticket as it worked for me on my end. I'll let you be the one to decide this since I don't know if you want any more testing, I entered as many weird numbers as I could and tried to crash it... it did not which is great.
In addition it worked as it was supposed to.
(0006181)
Dusk   
2013-03-28 17:48   
Quote from Torr Samaho
We still need to decide what to do with C_FlushDisplay().

I guess we should resolve this first?
(0006273)
Dusk   
2013-04-08 22:42   
con_notifylines confirmed to work by Arco, there's just the C_FlushDisplay problem now. What do?
(0006414)
Qent   
2013-06-11 01:54   
(edited on: 2013-06-11 01:55)
Make it optional with con_flushonlevelchange? I would like to keep the display uninterrupted (mostly for not missing chat messages online), but I could totally see someone wanting to keep a more vanilla feel (e.g. does it carry pickup messages into the intermission in singleplayer?).
(0006487)
Dusk   
2013-06-23 14:50   
I've split the C_FlushDisplay stuff into 0001377, since it's a different issue entirely. Since con_numnotifies works, I'll mark this as fixed.