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

Improve error messages in add-on manager and perhaps other places #2089

Open
Arcanister opened this Issue Oct 9, 2017 · 12 comments

Comments

Projects
None yet
4 participants
@Arcanister
Contributor

Arcanister commented Oct 9, 2017

When I scroll around add-on list in 1.13, I get error messages like this:

 error display: ~BLIT(): image not found: 'hits/blade-3.png'
 error display: unknown image function in path: DARKEN

The problem with the messages is that it's not clear which particular add-on icon produces the errors. It would be more helpful if the messages spit out complete image path instead of just error message and if they said what add-on name it was.

@jyrkive

This comment has been minimized.

Show comment
Hide comment
@jyrkive

jyrkive Oct 9, 2017

Member

Well, at the point where the error message is logged, the code has no idea where the image originates.

A more meaningful error message would require throwing an exception, and catching it at a point where the add-on is known. And adding an exception there is dangerous, because we may well forget to catch it somewhere, thus elevating the situation from a missing image into a crash.

Member

jyrkive commented Oct 9, 2017

Well, at the point where the error message is logged, the code has no idea where the image originates.

A more meaningful error message would require throwing an exception, and catching it at a point where the add-on is known. And adding an exception there is dangerous, because we may well forget to catch it somewhere, thus elevating the situation from a missing image into a crash.

@Arcanister

This comment has been minimized.

Show comment
Hide comment
@Arcanister

Arcanister Oct 9, 2017

Contributor

Is it possible to at least add complete image-path string such as picture1.png~BLIT(picture2.png)~SCALE(10,10) and/or somehow dump add-on list in a machine-parseable format, such as WML? Then, it will be possible to grep the dump for erroneous string such as "DARKENS".

Contributor

Arcanister commented Oct 9, 2017

Is it possible to at least add complete image-path string such as picture1.png~BLIT(picture2.png)~SCALE(10,10) and/or somehow dump add-on list in a machine-parseable format, such as WML? Then, it will be possible to grep the dump for erroneous string such as "DARKENS".

@jyrkive

This comment has been minimized.

Show comment
Hide comment
@jyrkive

jyrkive Oct 9, 2017

Member

Unfortunately, the modification chain isn't known at the time either. Only the current modification and its arguments are known.

You can dump the add-on list by printing response_buf.debug() into the log or a file here:

cfg = response_buf.child("campaigns");

Member

jyrkive commented Oct 9, 2017

Unfortunately, the modification chain isn't known at the time either. Only the current modification and its arguments are known.

You can dump the add-on list by printing response_buf.debug() into the log or a file here:

cfg = response_buf.child("campaigns");

@sevu

This comment has been minimized.

Show comment
Hide comment
@sevu

sevu Oct 9, 2017

Member

How about rejecting add-ons which have an invalid IPF as icon from being uploaded?
Similar like the other checks which have been discussed in the last days.

Member

sevu commented Oct 9, 2017

How about rejecting add-ons which have an invalid IPF as icon from being uploaded?
Similar like the other checks which have been discussed in the last days.

@Arcanister

This comment has been minimized.

Show comment
Hide comment
@Arcanister

Arcanister Oct 9, 2017

Contributor

By the way, I have found a easy way to get Wesnoth add-on list, requiring no programming knowledge and recompiling:

{ res="`echo -e '[request_campaign_list]\n[/request_campaign_list]' | gzip | base64`"; echo -en '\0\0\0\0'; dc -e "2 32 ^ `echo $res | base64 -d | wc -c` + P" | tail -c4; echo $res | base64 -d; } | nc addons.wesnoth.org 15008 | tail -c +9 | gunzip -c > addons.txt

This simple shell command will dump list of all add-ons from 1.13 server with their metadata into addons.txt

Contributor

Arcanister commented Oct 9, 2017

By the way, I have found a easy way to get Wesnoth add-on list, requiring no programming knowledge and recompiling:

{ res="`echo -e '[request_campaign_list]\n[/request_campaign_list]' | gzip | base64`"; echo -en '\0\0\0\0'; dc -e "2 32 ^ `echo $res | base64 -d | wc -c` + P" | tail -c4; echo $res | base64 -d; } | nc addons.wesnoth.org 15008 | tail -c +9 | gunzip -c > addons.txt

This simple shell command will dump list of all add-ons from 1.13 server with their metadata into addons.txt

@sevu

This comment has been minimized.

Show comment
Hide comment
@sevu

sevu Oct 9, 2017

Member

s/simple/complicated/

Member

sevu commented Oct 9, 2017

s/simple/complicated/

@CelticMinstrel

This comment has been minimized.

Show comment
Hide comment
@CelticMinstrel

CelticMinstrel Oct 9, 2017

Member

The DARKEN IPF was formerly valid though; Vultraz removed it at some point. I suppose we could add a compatibility layer to translate it to a blit though.

(Also yeah, that shell command is not in the least bit simple. BTW, it looks like the -R command in the wesnoth_addon_manager tool could also produce a similar result for you.)

Member

CelticMinstrel commented Oct 9, 2017

The DARKEN IPF was formerly valid though; Vultraz removed it at some point. I suppose we could add a compatibility layer to translate it to a blit though.

(Also yeah, that shell command is not in the least bit simple. BTW, it looks like the -R command in the wesnoth_addon_manager tool could also produce a similar result for you.)

@Arcanister

This comment has been minimized.

Show comment
Hide comment
@Arcanister

Arcanister Oct 10, 2017

Contributor

It would be wesnoth_addon_manager -lw > addons.txt but yes. Though shell command is still one line of code vs 420 lines of wesnoth_addon_manager.

Contributor

Arcanister commented Oct 10, 2017

It would be wesnoth_addon_manager -lw > addons.txt but yes. Though shell command is still one line of code vs 420 lines of wesnoth_addon_manager.

@CelticMinstrel

This comment has been minimized.

Show comment
Hide comment
@CelticMinstrel

CelticMinstrel Oct 11, 2017

Member

Calling that one line of code is seriously a stretch. Yes it's technically one line, but it's not readable as a one-liner.

Member

CelticMinstrel commented Oct 11, 2017

Calling that one line of code is seriously a stretch. Yes it's technically one line, but it's not readable as a one-liner.

@Arcanister

This comment has been minimized.

Show comment
Hide comment
@Arcanister

Arcanister Oct 11, 2017

Contributor
#!/bin/bash
# Send command necessary to retrieve addon-wml

server=addons.wesnoth.org
port=15008

gzip_command() {
	# base64 is necessary because env variable can't hold unguarded NUL
	gzipped="$(echo -ne "$1" | gzip | base64)"
	length="$(echo "$gzipped" | base64 -d | wc -c)"
	# convert length of command into uint32be
	dc -e "2 32 ^ $length + P" | tail -c4
	# append command itself
	echo -n "$gzipped" | base64 -d
}

# Send the command to server and skip first 8 bytes,
# which contain response to \0\0\0\0 and length of following gzipped data
{
	# Server expects 4 NUL sent before actual command
	echo -ne '\0\0\0\0'
	gzip_command '[request_campaign_list]\n[/request_campaign_list]\n'
} 	| nc "$server" "$port" | tail -c +9 | gunzip -c

# EOF
Contributor

Arcanister commented Oct 11, 2017

#!/bin/bash
# Send command necessary to retrieve addon-wml

server=addons.wesnoth.org
port=15008

gzip_command() {
	# base64 is necessary because env variable can't hold unguarded NUL
	gzipped="$(echo -ne "$1" | gzip | base64)"
	length="$(echo "$gzipped" | base64 -d | wc -c)"
	# convert length of command into uint32be
	dc -e "2 32 ^ $length + P" | tail -c4
	# append command itself
	echo -n "$gzipped" | base64 -d
}

# Send the command to server and skip first 8 bytes,
# which contain response to \0\0\0\0 and length of following gzipped data
{
	# Server expects 4 NUL sent before actual command
	echo -ne '\0\0\0\0'
	gzip_command '[request_campaign_list]\n[/request_campaign_list]\n'
} 	| nc "$server" "$port" | tail -c +9 | gunzip -c

# EOF
@CelticMinstrel

This comment has been minimized.

Show comment
Hide comment
@CelticMinstrel

CelticMinstrel Oct 12, 2017

Member

Still not a one-liner. :P

Member

CelticMinstrel commented Oct 12, 2017

Still not a one-liner. :P

@sevu

This comment has been minimized.

Show comment
Hide comment
@sevu

sevu Oct 14, 2017

Member

But it has comments!

Member

sevu commented Oct 14, 2017

But it has comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment