[Commits] [wesnoth/wesnoth] 1df298: campaignd: Send uploading clients a list of illega...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/wesnoth/wesnoth
  Commit: 1df298511881e30bdab2ff9a473631dd1dc2dfb0
      https://github.com/wesnoth/wesnoth/commit/1df298511881e30bdab2ff9a473631dd1dc2dfb0
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2017-10-05 (Thu, 05 Oct 2017)

  Changed paths:
    M src/addon/client.cpp
    M src/addon/client.hpp
    M src/addon/validation.cpp
    M src/addon/validation.hpp
    M src/campaign_server/campaign_server.cpp
    M src/campaign_server/campaign_server.hpp
    M src/gui/dialogs/addon/manager.cpp

  Log Message:
  -----------
  campaignd: Send uploading clients a list of illegal names when any are found

(See issue #2043.)

It wouldn't be proper to stuff the full list into the conventional error
response, so in order to provide clients with the means to report it in
a better fashion in the future (a la WML load errors report dialog, with
a fancy scrolling box and an option to copy the report to clipboard and
stuff), we're attaching it as an extra_data attribute in the [error]
response. Naturally, only clients that are aware of its existence can
handle it, so this whole patch is completely unsuitable for 1.12.

Still, some level of backwards compatibility is retained -- the [error]
continues to be perfectly functional for incompatible clients (e.g.
1.13.10), and the details are simply missing for them.

The list would currently look like this:

  Test_Addon_420/~illegalname
  Test_Addon_420/testdir/illegal name 2
  Test_Addon_420/testdir/illegal~dir~name/

The required client-side changes for handling the extra_data attribute
and displaying its contents are included in this patch, but the GUI
changes are very rudimentary and should be considered more of a
proof-of-concept than a final solution, which I'll leave to someone who
actually groks GUI2 as it is right now and is an active developer in
the first place, which I am not.

Also note that the only reason the second parameter for
check_names_legal() is optional is that the function is also used
client-side by addons_client::install_addon(), which really doesn't need
to waste time building a full report of invalid names -- add-ons
containing those aren't supposed to make it to the server in the first
place, and the client-side check only exists as an extra safeguard.