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

No option to disable online version check on startup #597

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
8 participants
@nvx
Copy link

commented Jan 23, 2013

There is no configurable option to disable the online version check on startup, adding many seconds to the startup time, and utterly useless when BuildCraft is included in server mod packs/etc.

Please either remove it entirely, or at the very least add a config option to disable it.

@SirSengir

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2013

"NVX> dear anyone who writes mods, if you decide to include an update check plugin, stupidly do it in the main thread and you decide to not include a config option to disable it, please kill yourself - thanks"

Submit a PR or STFU.

@SirSengir SirSengir closed this Jan 23, 2013

@SirSengir

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2013

You didn't fix it, you just removed it.

@LiaungYip

This comment has been minimized.

Copy link

commented Jan 23, 2013

@SirSengir: Sounds fixed to me.

@Krapht

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2013

A fix in this case would be to add the config option or to make it threaded, not removing it

@nevercast

This comment has been minimized.

Copy link

commented Jan 23, 2013

A fix would actually be both of those.
Any optional ( In the context of not being required to make the game work, authentication would be an except for example ) web requests SHOULD be in another thread. There should also be a config option for this.

With that said, I don't give a shit really, as it doesn't delay my start up by much more than any of the other mods do, so I've no PR for this.

Just saying that if @nvx wants to provide a real fix, it should be both threaded and config option.

@nvx

This comment has been minimized.

Copy link
Author

commented Jan 24, 2013

Now that I can comment again (thanks for the block SirSengir, it means a lot to me!) - A real fix would be more than the lines I removed, it would have also nuked it from Version.java and a few other places.

Update check features are useless in this developers humble opinion, especially ones that block the main thread during startup. On my rather fast connection at home it delays startup by 3 seconds. I daresay on your average connection if you're doing a few things in the background that number would increase a lot, not to mention if you don't have Internet connectivity at all playing in offline mode you'd have to wait for the connection attempt to time out.

To put those numbers into perspective, Tekkit Lite has around 40 mods listed, and FTB has around 20 mods listed. I'm assuming the latter actually has closer to 40, but just doesn't list as many individual mods. I haven't really looked, feel free to look yourself if you're interested. What this means though is if every mod decided to take 'just 3 seconds' of extra start up time for an utterly useless feature, it would delay Minecraft loading by an ADDITIONAL 2 minutes.

Is this acceptable? No. Is it acceptable to have a reply of "PR or STFU" followed by CLOSING of a perfectly valid issue? No. Open source does not mean you can tell everyone to fix your own screwups. An update check is useless to me, I'm not going to "improve" it by throwing it in another thread, adding a config option. I'll just remove it. That solves the issue for me. If you want a better fix, do it yourself - and don't close perfectly valid issues just because you have a petty grudge with the opinions of the bug reporter.

@ghost

This comment has been minimized.

Copy link

commented Jan 24, 2013

I think you wait for 2 minutes. It's not that bad.

@nvx

This comment has been minimized.

Copy link
Author

commented Jan 24, 2013

@pizzana, some people didn't mind Windows ME either - doesn't mean it was good. Please try to keep it relevant.

@psxlover

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2013

Forcing someone else's mod to work the way you want is isn't a valid PR. This PR is no better than the one in forge for the x-ray mod...

@nvx

This comment has been minimized.

Copy link
Author

commented Jan 24, 2013

@psxlover, Did you miss the part where I got told "PR or STFU" and the issue closed? The PR was simply a response to that. I'm not going to waste my time fixing a broken implementation of a useless feature that I for one would never have enabled. @SirSengir has a view of open source software that bug reports are invalid unless you're willing to fix them yourself apparently - so I fixed it the way I personally would have fixed it if it was a code base I was managing at the time - by removing said 'feature'.

It is my opinion that removing said 'feature' is a better solution than leaving it as-is in its current state. One could argue a better fix would be to thread it and offer a config option - sure, do that if you like - but short of that you can't possibly argue that it's better to have this 'feature' exist in its current form than to have it not exist at all - and that's what I did.

@SirSengir of course could have written his own 'better' fix, but alas he chose to just close the bug report. Clearly this is an intended 'feature' of BuildCraft nowadays. Perhaps version 3.5.0 will block the main thread by sleeping for 30 seconds during start up for fun too.

@Krapht

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2013

If you look at what Sengir quoted from IRC you understand why you got the reply you got. Also, open source does not mean it is a democracy, nor does it mean that you always get your way. Last I checked, Sengir and I are in charge of BuildCraft, and we have final say in what does and what does not go into the mod.

With regards to this "bug", I would not define it as a bug. At best it would be lack of feature. I agree that threading and maybe even a config option is a feature worth implementing, but at the moment there are more pressing issues to work on. So if you're in a hurry to get this "issue" fixed, submitting a PR is the best way of doing it. Or you can simply remove the feature you don't like, compile your own jar and use that. That is the joy of open source.

@nvx

This comment has been minimized.

Copy link
Author

commented Jan 24, 2013

What I said in IRC was not directed at Sengir or BuildCraft specifically (he was not even active in IRC at the time), nor was it in a BuildCraft channel. It was a general expression of distaste for software in general that performs actions behind the users back without any option to disable it that also directly negatively affects the user experience.

So blocking the main thread without warning, without notification to the user as to what's happening, and without any option to change the behaviour, for several seconds (potentially up to a minute if network connectivity isn't available depending on operating system and the exact circumstances) all for the sake of a line in a log file isn't what you consider to be a bug?

I'm not in a 'hurry' to get this fixed, I never said "fix it now zomgs!" (feel free to take that out of context too if you'd like). My main objections were "PR or STFU" followed by closing of the issue - the fact that he requested a PR means he has acknowledged that it is in fact an issue, yet he closed the issue report purely out of spite. It's not a democracy, sure - but when someone decides to be an arse purely out of spite, I'm going to call them out on it.

Edit: This goes below SirSengir's post:

I'll be sure to note in the future that you can't take a joke. The "whining" that followed was about mods that had update check features in general, as well as mods that don't allow disabling of said features, and mods that do it in the main thread. It just so happens that BC3 ticked all of these boxes. Another highlight of the "whining" was discussion of a better way to keep track of mod versions (done in a mod-managing launcher), updates etc. But you already know this, you were in the channel.

If it would make your ego feel any better I can also state for the public record that I am also displeased with OptiFine's automatic updating that has no option to disable (however the authors were at least considerate enough to defer it to another thread), Industrial Craft 2's fetching of http://rg.dl.je/jzYcbjmOP2Y947yVCOX37EFnlxuXhj.txt for capes for the mod authors (surely this ego stroking could have been baked into the jar) - but still at least considerate enough to defer to another thread.

But, since we're discussing what was said in irc, and considering your stance of "I do not consider suggesting people to go kill themselves over such a trivial issue even remotely acceptable behaviour. Even on the internet." I thought the following gem you said in IRC would be appropriate.

<+SirSengir> NVX, if you decide to bitch about the property of an open source project instead of submitting a PR to fix it, please kill yourself.

I'm not even sure where I was bitching about "property", but a tad hypocritical none the less in any case. If you have any issues with me personally, I suggest taking them up on IRC (you know where to find me) rather than further derailing this issue report and until such a time as a time as a fix has been committed to BuildCraft, or you are deciding that this is a known desired intended feature that will not be fixed (and thus not accepting fixes for it) to re-open this issue.

@SirSengir

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2013

I'll quote your line from a public IRC channel with ~300 people in it again:

"NVX> dear anyone who writes mods, if you decide to include an update check plugin, stupidly do it in the main thread and you decide to not include a config option to disable it, please kill yourself - thanks"

This directly following whining about BC's lack of threading of the version check. Perhaps I am too old, but I do not consider suggesting people go kill themselves over such a trivial issue even remotely acceptable behaviour. Even on the internet.

I was still prepared to accept a proper PR that includes a config option to disable the check. You had that chance. Would have taken you ~ 4 - 5 lines more of code.

Instead you chose to follow your up your faux-pas with a PR that clearly spells "frack you" in so many lines of code. You had the chance to fix it with a proper PR instead you chose passive-aggressive grandstanding.

Well, this is the internet, so you can say "if you can't stand the heat, get out of the kitchen". Same goes the other way around: If you choose to just be difficult, you'll have to deal with getting ignored/your comments deleted/blocked.

Edit: Also learn to spell "rhetorical device" before throwing around the word hypocrisy.

@Krapht

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2013

This is why we can't have nice things... Turning off issues until the spamming stops.

@Myrathi

This comment has been minimized.

Copy link

commented Jan 24, 2013

I can only assume nvx's reply is being consistently reposted and deleted, so: seriously, nvx... give it a damned rest!

I didn't care much for this "discussion" in the first place (your discontent at being "muted" in a place where you have no actual "right" to an opinion is moot) but your consistent spamming of MY inbox is beyond a joke and all your doing is making yourself look like a bigger and bigger ass and diluting any credibility you may (not) have had in the first place.

Step off and grow up, nvx. Sheesh.

@pahimar

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2013

This is my fault, since I said I would fix this with a threaded version a week ago. Look for some commits on this later today.

@nvx

This comment has been minimized.

Copy link
Author

commented Jan 24, 2013

Not your fault pahimar for the trainwreck by any stretch, but I look forward to your commit! :)

Edit: Apologies for all the notifications Myrathi, turns out I was the only one -not- receiving the notifications each time I undid Sengir's censoring so I didn't realise.

@pahimar

This comment has been minimized.

Copy link
Contributor

commented Jan 26, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.