Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The physical blackbox object no longer stores feedback data #23419

Merged
merged 3 commits into from
Feb 4, 2017
Merged

The physical blackbox object no longer stores feedback data #23419

merged 3 commits into from
Feb 4, 2017

Conversation

ChangelingRain
Copy link
Contributor

instead it's just a datum and shouldn't have multiples or fuck up

@ChangelingRain ChangelingRain added the Fix Rewrites a bug so it appears in different circumstances label Jan 28, 2017
@Fox-McCloud
Copy link
Contributor

How this piece of crap survived as long as it did is beyond me.

Also, who in the hell thought it was a good idea to make this an in world actual object as opposed to a controller or datum to begin with?

@Jordie0608
Copy link
Member

Take a guess e6aa3fc
Bonus points for being relative pathed.
To be fair it was 6 years ago coding standards weren't really a thing nor was compiling your code.

@MrStonedOne
Copy link
Member

How this piece of crap survived as long as it did is beyond me.

Also, who in the hell thought it was a good idea to make this an in world actual object as opposed to a controller or datum to begin with?

The entire point was to make it meme well with "black box" as an object. the idea being that all in game stats go thru the stations blackbox recorder as a nice easteregg.

And its lasted 6 years because every time somebody tries to change it, I'm in here, defending it.

@optimumtact
Copy link
Member

MSO this isn't tablecrafting and in this case it's a bit silly to have our feedback relying on a physical object that other people insist on breaking all the time.

@MrStonedOne
Copy link
Member

Actually, this has only been broken once.

When qdel was added.

This wasn't a new bug at all, it was just that del(src) also acts as a return and that wasn't changed when it was changed to qdel()

The bug existed since then.

@MrStonedOne
Copy link
Member

Not to mention that I would love to make an ability to actually delete this object, and i mean truly delete it, to wipe all stats for the round.

@MrStonedOne
Copy link
Member

Like that can be a feature.

@MrStonedOne
Copy link
Member

And the idea that it would be a facade, and it would really just flip some switch to not report them feels cheap.

@MrStonedOne
Copy link
Member

If i added such a feature, what do you think players would find more cool?

"doing it just tells the game not to report the stats"

"doing it actually deletes the stats because the black box recorder actually is involved in storing/tracking the stats"

It's a subtle difference, but one is more cool.

@nfreader
Copy link
Contributor

Not to mention that I would love to make an ability to actually delete this object, and i mean truly delete it, to wipe all stats for the round.

Just so long as it's clear what happened to that round in the DB so I can work around it. It should, at the very least, report server, map, start time, end time, and then throw a flag that the blackbox was disabled/destroyed.

@coiax
Copy link
Member

coiax commented Jan 28, 2017

@MrStonedOne can we change the blackbox to something linked to the global datum, rather than storing it. Then the blackbox can be a reference, and have an admin switch or something. I always liked the idea of Centcom full of "admin machinery".

@optimumtact
Copy link
Member

no because disabling something we are ostensibly using to make game balance decisions is a stupid idea.

mso please, you won, slap crafting is still in the game, there's no need for this

@MrStonedOne
Copy link
Member

I haven't won because people still think removing the blackbox as the tracker of feedback stats is a good thing.

@ChangelingRain
Copy link
Contributor Author

Wow, look, petty!
Hey, guess what, you can delete the feedback data if you want, probably via sqdl or something, but that's fucking stupid.

I want stat logging even if it's from stupid admin bullshit; at worst, maybe a toggle to mark a round as admin bullshit so you can filter that out if you want, but I don't want to lose stat info.

…blackboxed

Conflicts:
	code/modules/research/message_server.dm
@ChangelingRain
Copy link
Contributor Author

I haven't won because people still think removing the blackbox as the tracker of feedback stats is a good thing.

with that as your opinion you aren't going to win

@MrPerson
Copy link
Contributor

MrPerson commented Jan 28, 2017

Why would we bother gathering stats and then not report them? Add some sort of IC use for the stats and then filter out lost rounds from that. Our stats should record all rounds no matter what, period.

@Iamgoofball
Copy link
Contributor

Iamgoofball commented Jan 29, 2017

compromise

the PR stays as-is, making sure our back end tracking stays solid and we can properly balance, and all our stat tracking stuff isn't wasted because admins wanted to hide their stupid bullshit

the blackbox becomes a tracker of generic stuff on station similar to what it does now, but it's used to compile a round-end report displayed to players at the end of the round, and can be accessed from in game via a button or something on the lobby screen showing past rounds

if it's destroyed, specific data becomes unavailable and only the most basic information is available from the lobby and round-end report

@ChangelingRain
Copy link
Contributor Author

ChangelingRain commented Jan 29, 2017

As long as you always get stat info, OOC, I don't care what you do with the black box.

@optimumtact
Copy link
Member

Same opinion as @ChangelingRain here

…blackboxed

Conflicts:
	code/orphaned_procs/statistics.dm
@optimumtact optimumtact merged commit af25652 into tgstation:master Feb 4, 2017
@ChangelingRain ChangelingRain deleted the blackboxed branch February 4, 2017 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Rewrites a bug so it appears in different circumstances
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants