GlowstoneMC / Glowstone-Legacy

An open-source server for the Bukkit Minecraft modding interface
Other
363 stars 122 forks source link

Unintentionally broke MultiCraft (and some hosting provider) support #586

Closed turt2live closed 9 years ago

turt2live commented 9 years ago

Now that Glowstone has a warning for not having a console (see 23a4b8237915d49c67f259e3e33a8f3f6cee19ca) Multicraft no longer functions.

This is due to Multicraft starting the jar in a process and not as a console application. Many other hosts also provide a similar starting system and therefore do not have console access.

Ideally, commit 23a4b8237915d49c67f259e3e33a8f3f6cee19ca would have been tested more before being thrown into master to avoid the lack of research that was stated in #581.

My personal suggestion is to implement a command line argument to prevent the warning. In addition, I would suggest to implement a check to determine if an X server (or similar) is available, if one is not then simply printing the warning before exiting would be best. Many hosts and panels, such as Multicraft, still provide a tail of the log and therefore a printed message would be suitable.

bendem commented 9 years ago

I don't really think the default behavior should be to prevent the server from starting. Most of the server will be run from hosts taking care of that part or on a dedicated server (thus launched using the console anyway).

Using a flag to run glowstone anyway wouldn't make sense either. If the host doesn't provide a console nor an X server, they won't provide a way to add startup flags.

IMHO, preventing glowstone to run if no console is found is not the right call.

My personal suggestion is to implement a command line argument to prevent the warning. In addition, I would suggest to implement a check to determine if an X server (or similar) is available, if one is not then simply printing the warning before exiting would be best.

If no X server has been found, I don't think there is a need to prevent the server from starting.

mastercoms commented 9 years ago

I think it should warn the user, both using the GUI window and in the console log, but start the server anyway.

turt2live commented 9 years ago

The point of this ticket is to make note of a failure regarding what was implemented due to #581, not to re-discuss the topic in #581.

Although in my opinion #581 was resolved too quickly this is not the place to re-discuss the problem scope of another ticket.

SpaceManiac commented 9 years ago

Okay, I've disabled it for now until we can work out a specific way to detect the console situation in a way that doesn't ruin anything for weird console environments.

mastercoms commented 9 years ago

@turt2live So we should discuss in #581?

turt2live commented 9 years ago

@mastercoms My personal opinion would be to discuss it on the forums instead. Reviving dead tickets is not a good idea.

mastercoms commented 9 years ago

@turt2live Alright.

bendem commented 9 years ago

in a way that doesn't ruin anything for weird console environments

I don't think it is that weird, the best would be to ask people running servers or server hosts though.

SpaceManiac commented 9 years ago

The suggested System.console() == null && !GraphicsEnvironment.isHeadless() && Desktop.isDesktopSupported() fix is promising, especially for server host environments, but on a desktop Windows machine I still can't java -jar glowstone.jar | tee output.txt without the check passing. I'm considering checking args.length == 0 also, since this should only be true for a double-clicked jar, but it might sometimes be true for a console run. I don't see any way to detect javaw outright on Windows, and I'm not sure Mac has a separate executable (Linux certainly does not). Thoughts?

SpaceManiac commented 9 years ago

I'm tempted to just abandon the console check since it seems like there's no reasonable way to show the warning message in the desired situation without having some false positives. I suppose more conditions could be added, like "only on Windows" or "no command-line arguments specified", but this just seems hacky, and for relatively little benefit (a one-time benefit to users who don't read the install instructions). Thoughts?

bendem commented 9 years ago

Agreed

dequis commented 9 years ago

Maybe just improve the "failed to bind" message to include a few lines of instructions telling users what happened and how to fix it? Because that's the main problem, really.

And maybe, MAYBE, something that looks less like a stack trace (which is commonly associated with bugs)