Don't print lobby.buddies to mainlog.html
ClosedPublic
Actions

Authored by elexis on Jun 4 2018, 11:13 PM.

Details

Reviewers: None
Commits: rP21844: Don't print lobby buddies to mainlog.html, refs D209 / rP19433.

Summary

<p>Loaded config string "lobby.buddies" = "Boudica,Emperior,fpre,temple,user1,mapkoc,B_Smoke,Franksy,RetardadoSVN,bbleft,Dizaka,Lion_Kanzen,Feldfeld,Grugnas,Pretension,Uran238cz,echotangoecho,PhyZic,fsvn,merlin1,OptimusShepard,Retardado,Achelao,fatherbushido,realNoobNoob,thenu,Bataar,kjager,Cesar,borg-,ycswyw,ffm,maxticatrix,kristian,soloooyo,bb,Merov,nigel87,FFFFFFF8,mord,Cool_Guy,lololo,drunkadius,causative,Liberty,samba,mo,Predisposition,kizitom,leper__,_zoro_,TrumpBurger,santa,Imarok,Hannibal_Baraq,hannibal_baraq,PrincipalityOfZeon,Tiber7,Hannibal_Barca,KonorMakGreGore,Imarok,Cesar,borg-,ycswyw,ffm,maxticatrix,kristian,soloooyo,bb,Merov,nigel87,FFFFFFF8,mord,Cool_Guy,lololo,drunkadius,causative,Liberty,samba,mo,Predisposition,kizitom,Hannibal_Barca,chrstgtr,mars,inkomen"</p>

Just like the lobby password, this shouldn't be printed to the logfile.

Test Plan

UserReport.id too? Networking password not implemented yet, but equally shouldn't be printed.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Jun 4 2018, 11:13 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/628/display/redirect

Harbormaster completed remote builds in B6216: Diff 6723.Jun 4 2018, 11:18 PM

vladislavbelov added a subscriber: vladislavbelov.Jun 5 2018, 1:20 AM

vladislavbelov added inline comments.

source/network/NetMessages.h
124 ↗	(On Diff #6723)	Maybe `TODO:` then?
source/ps/ConfigDB.cpp
370 ↗	(On Diff #6723)	Shouldn't it be declared in the JS part? Since we can add options in mods.

elexis added inline comments.Jun 5 2018, 1:39 PM

source/ps/ConfigDB.cpp
370 ↗	(On Diff #6723)	Options are barely supported for mods. https://github.com/fraizy22/fgodmod/ is forced to provide a modified `default.cfg`. But if mods have to do that, they each replace that file. We have no place to put this, `options.json` is not for all config entries, `default.cfg` only stores config defaults. For the hotkey edit dialog I will need a JSON file too specifying all config entries. It were really better to have a JSON config manifest and group all information there, but needs someones lifetime.
371 ↗	(On Diff #6723)	Any opinion on the userreporterID? I think we can keep it as it doesn't reveal anything private or secret if its not combined with the UserReporter db and if the UserReporter has any use case, it would apply to debugging devs reading mainlogs.

Use std::map, +1 TODO, -1 TODO

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/631/display/redirect

Harbormaster completed remote builds in B6219: Diff 6726.Jun 5 2018, 2:15 PM

vladislavbelov added inline comments.Jun 6 2018, 3:31 AM

source/ps/ConfigDB.cpp
37 ↗	(On Diff #6726)	Why without the `g_` prefix? Since it's the global variable.
371 ↗	(On Diff #6723)	I think currently the `UserReport.ID` isn't needed in logs. Because reports are used for HW stats, but we already receive them with the `system_info` file.

elexis added inline comments.Jun 6 2018, 8:55 PM

source/ps/ConfigDB.cpp
37 ↗	(On Diff #6726)	Can change since it's in the coding conventions currently. But its benefitial if there is a way to find globals that are used across files (those are worst globals)
371 ↗	(On Diff #6723)	It's not really a quesiton if its needed or not, but only if it reveals private information or not. Hardware stats can help debugging too if it's a hardware or driver problem and the user might not always have uploaded the `system_info.txt` and can become unavailable (average case in fact).