Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1625502-tips-in-help into lp:widelands
Let's get this branch in so we can add the content changes in another branch. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1625502-tips-in-help/+merge/312969 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1625502-tips-in-help. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/post_django1_8_cleanup into lp:widelands-website
Ups, forgot that one. -- https://code.launchpad.net/~widelands-dev/widelands-website/post_django1_8_cleanup/+merge/315301 Your team Widelands Developers is subscribed to branch lp:widelands-website. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/update_copyright_script into lp:widelands
Thanks for the review - added replies to your comments.
Diff comments:
>
> === added file 'utils/file_utils.py'
> --- utils/file_utils.py 1970-01-01 00:00:00 +
> +++ utils/file_utils.py 2017-01-23 12:35:34 +
> @@ -0,0 +1,32 @@
> +#!/usr/bin/env python
> +# -*- coding: utf-8 -*-
> +
> +
> +"""Some common file util functions."""
> +
> +import os
> +import sys
> +
> +PYTHON3 = sys.version_info >= (3, 0)
> +
> +def read_text_file(filename):
> +"""Reads the contens of a text file."""
> +if PYTHON3:
> +return open(filename, 'r', encoding='utf-8').read()
> +else:
> +return open(filename, 'r').read().decode('utf-8')
They should be - it would be a bug anyway otherwise. I think it's not a problem
anyway unless the files contain non-ASCII characters, which would be in
localizeable strings only. If that should ever happen, our translators will
report the bug pretty quickly.
> +
> +
> +def write_text_file(filename, content):
> +"""Writes 'content' into a text file."""
> +if PYTHON3:
> +open(filename, 'w', encoding='utf-8').write(content)
> +else:
> +open(filename, 'w').write(content.encode('utf-8'))
> +
> +
> +def find_files(startpath, extensions):
> +for (dirpath, _, filenames) in os.walk(startpath):
> +for filename in filenames:
> +if os.path.splitext(filename)[-1].lower() in extensions:
> +yield os.path.join(dirpath, filename)
>
> === added file 'utils/update_copyright.py'
> --- utils/update_copyright.py 1970-01-01 00:00:00 +
> +++ utils/update_copyright.py 2017-01-23 12:35:34 +
> @@ -0,0 +1,54 @@
> +#!/usr/bin/env python
> +# encoding: utf-8
> +
> +import os.path
> +import re
> +import sys
> +
> +file_utils_script = os.path.abspath(os.path.join(os.path.dirname(__file__),
> 'file_utils.py'))
> +exec(compile(source=open(file_utils_script).read(),
> filename=file_utils_script, mode='exec'))
> +
> +def main():
> +"""Updates the copyright year in all source files to the given year."""
> +if len(sys.argv) != 2:
> +print('Usage: update_copyright.py ')
> +return 1
> +
> +try:
> +year = sys.argv[1]
> +sys.stdout.write('Updating copyright year to: ' + year + ' ')
> +src_path = os.path.abspath(os.path.join(os.path.dirname(__file__),
> "../src"))
> +# Fix copyright headers in C++ files
> +for filename in find_files(src_path, ['.h', '.cc']):
> +sys.stdout.write('.')
> +sys.stdout.flush()
> +lines = read_text_file(filename).strip().split('\n')
> +new_lines = []
> +regex = re.compile('(.*Copyright \(C\) \d\d\d\d)(.*)( by the
> Widelands Development Team.*)')
> +for line in lines:
> +match = regex.match(line)
> +if match:
> +line = match.group(1) + "-" + year + match.group(3)
> +new_lines.append(line.rstrip() + '\n')
> +write_text_file(filename, ''.join(new_lines))
I don't think it's likely that in-line comments will contain "... Copyright (C)
... by the Widelands Development Team. ..." And if they do, they will simply
get their copyright years updated without removing the code, because the regex
starts and ends with .*
> +
> +# Now update the Buildinfo
> +filename = os.path.join(src_path, "build_info.h")
> +lines = read_text_file(filename).strip().split('\n')
> +new_lines = []
> +regex = re.compile('(.*constexpr uint16_t kWidelandsCopyrightEnd =
> )(\d\d\d\d)(;)')
> +for line in lines:
> +match = regex.match(line)
> +if match:
> +line = match.group(1) + year + match.group(3)
> +new_lines.append(line.rstrip() + '\n')
> +write_text_file(filename, ''.join(new_lines))
> +print(' done.')
> +
> +except Exception:
> +print('Something went wrong:')
> +traceback.print_exc()
> +return 1
> +
> +if __name__ == '__main__':
> +sys.exit(main())
--
https://code.launchpad.net/~widelands-dev/widelands/update_copyright_script/+merge/315352
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/update_copyright_script into lp:widelands.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/switch_direction into lp:widelands
I'll give it a spin to see how it feels. If we can't get this to work without making me motion sick, we'll have to keep the smooth scrolling for the scenarios only and simply jump like we used to. The way it is now, the feature is blocking me from using the minimap. -- https://code.launchpad.net/~widelands-dev/widelands/switch_direction/+merge/315432 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/switch_direction. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/build_docs_on_travis into lp:widelands
Review: Approve Code LGTM. We should have branches ready to fix the errors before merging though, because this will make Travis unusable for all other branches in the meantime. -- https://code.launchpad.net/~widelands-dev/widelands/build_docs_on_travis/+merge/315431 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/build_docs_on_travis. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1625502-tips-in-help into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1625502-tips-in-help into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1625502-tips-in-help/+merge/312969 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1625502-tips-in-help. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/glossary_checks into lp:widelands
Continuous integration builds have changed state: Travis build 1864. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/194595810. Appveyor build 1700. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_glossary_checks-1700. -- https://code.launchpad.net/~widelands-dev/widelands/glossary_checks/+merge/312430 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/glossary_checks. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/switch_direction into lp:widelands
Review: Approve Gave it a quick spin - my visual cortex can handle this version. So, it's either this, or jump. -- https://code.launchpad.net/~widelands-dev/widelands/switch_direction/+merge/315432 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/switch_direction. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/update_copyright_script into lp:widelands
Maybe a silly question: Looks like the files are opened but not closed? Python programmers using mostly the 'with' statement when handling files, which automatically closes files. See the last example in https://docs.python.org/2/tutorial/inputoutput.html#methods-of-file-objects -- https://code.launchpad.net/~widelands-dev/widelands/update_copyright_script/+merge/315352 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/update_copyright_script into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/switch_direction into lp:widelands
Have you tried modifying the parameters in map view.cc? Maybe you find better ones. > Am 24.01.2017 um 09:18 schrieb GunChleoc : > > Review: Approve > > Gave it a quick spin - my visual cortex can handle this version. So, it's > either this, or jump. > -- > https://code.launchpad.net/~widelands-dev/widelands/switch_direction/+merge/315432 > You proposed lp:~widelands-dev/widelands/switch_direction for merging. -- https://code.launchpad.net/~widelands-dev/widelands/switch_direction/+merge/315432 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/switch_direction. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/switch_direction into lp:widelands
Continuous integration builds have changed state: Travis build 1865. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/194738386. Appveyor build 1702. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_switch_direction-1702. -- https://code.launchpad.net/~widelands-dev/widelands/switch_direction/+merge/315432 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/switch_direction. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/build_docs_on_travis into lp:widelands
Continuous integration builds have changed state: Travis build 1866. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/194739060. Appveyor build 1703. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_build_docs_on_travis-1703. -- https://code.launchpad.net/~widelands-dev/widelands/build_docs_on_travis/+merge/315431 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/build_docs_on_travis. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/update_copyright_script into lp:widelands
Review: Approve test, use Testing is all fine update_copyright_script$ ./utils/update_copyright.py Usage: update_copyright.py brombeere:update_copyright_script $ ./utils/update_copyright.py 2017 Updating copyright year to: 2017 ... lots of dots ... done. brombeere:update_copyright_script $ bzr diff .. lots of changes like ... - * Copyright (C) 2006, 2008-2010 by the Widelands Development Team + * Copyright (C) 2006-2017 by the Widelands Development Team all sane /usr/bin/env python -> Python 3.4.6 Tried with python2.6 and python2.7 with the same result. I did no re-(test) fix_formatting.py. Is this applied automatically or manually? -- https://code.launchpad.net/~widelands-dev/widelands/update_copyright_script/+merge/315352 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/update_copyright_script. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/build_docs_on_travis into lp:widelands
I am giving up on this branch. We'd require sphinx 1.4 to build our docs, but travis only has 1.2. I am not willing to put in more time to fix this. -- https://code.launchpad.net/~widelands-dev/widelands/build_docs_on_travis/+merge/315431 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/build_docs_on_travis. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/build_docs_on_travis into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/build_docs_on_travis into lp:widelands has been updated. Status: Needs review => Rejected For more details, see: https://code.launchpad.net/~widelands-dev/widelands/build_docs_on_travis/+merge/315431 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/build_docs_on_travis. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/update_copyright_script into lp:widelands
@kaputtnik: I found no way of explicitly closing the files that works while keeping the wrapper functions. All I get is errors, even .close() won't work. I also find the code to include the file utils script ridiculously ugly, do you know a more elegant way of doing that? @Haso50: fix_formatting can be run manually, but it's also run by bunnybot, and when updating translations. I only touched it to pull out the file functions that I reused. -- https://code.launchpad.net/~widelands-dev/widelands/update_copyright_script/+merge/315352 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/update_copyright_script. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1191295-no-seafaring-builder into lp:widelands
Notabilis has proposed merging
lp:~widelands-dev/widelands/bug-1191295-no-seafaring-builder into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1191295 in widelands: "Seafaring: builder not listed in expedition list
in port"
https://bugs.launchpad.net/widelands/+bug/1191295
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1191295-no-seafaring-builder/+merge/315525
Added display of builder to expedition bootstrap. Seems to work fine so far,
but I am not sure how to implement the saving/loading code (in the branch
disabled). The current version of it does not contain a version number so
changing it will probably break it.
Possible hack:
Currently the first entry in the saved bootstrap class is the number of
builders already waiting. Since this is always 0 or 1 this could be abused as a
"version number" when loading the packet, using version number 2 for the new
packet format. This should work for loading older save games with a new version
of the game but would give some strange error/crash when loading a new save
game with an old version of the game (I guess. Since the old game would not
recognize the new format it would try to load two workers).
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1191295-no-seafaring-builder into lp:widelands.
=== modified file 'src/economy/expedition_bootstrap.cc'
--- src/economy/expedition_bootstrap.cc 2016-12-05 09:24:10 +
+++ src/economy/expedition_bootstrap.cc 2017-01-24 23:24:19 +
@@ -24,6 +24,8 @@
#include "base/macros.h"
#include "base/wexception.h"
#include "economy/portdock.h"
+#include "economy/wares_queue.h"
+#include "economy/workers_queue.h"
#include "io/filewrite.h"
#include "logic/map_objects/tribes/warehouse.h"
#include "logic/player.h"
@@ -33,35 +35,17 @@
namespace Widelands {
-struct ExpeditionBootstrap::ExpeditionWorker {
- // Ownership is taken.
- ExpeditionWorker(Request* r) : request(r), worker(nullptr) {
- }
-
- std::unique_ptr request;
-
- // we never own the worker. Either he is in the PortDock in which case it
- // owns it or it is on the ship.
- Worker* worker;
-};
-
ExpeditionBootstrap::ExpeditionBootstrap(PortDock* const portdock)
: portdock_(portdock), economy_(portdock->get_economy()) {
}
ExpeditionBootstrap::~ExpeditionBootstrap() {
- assert(workers_.empty());
- assert(wares_.empty());
+ assert(queues_.empty());
}
void ExpeditionBootstrap::is_ready(Game& game) {
- for (std::unique_ptr& wq : wares_) {
- if (wq->get_max_fill() != wq->get_filled())
- return;
- }
-
- for (std::unique_ptr& ew : workers_) {
- if (ew->request)
+ for (std::unique_ptr& iq : queues_) {
+ if (iq->get_max_fill() != iq->get_filled())
return;
}
@@ -70,48 +54,13 @@
}
// static
-void ExpeditionBootstrap::ware_callback(
- Game& game, InputQueue*, DescriptionIndex, Worker*, void* const data) {
+void ExpeditionBootstrap::input_callback(Game& game, InputQueue*, DescriptionIndex, Worker*, void* data) {
ExpeditionBootstrap* eb = static_cast(data);
eb->is_ready(game);
}
-// static
-void ExpeditionBootstrap::worker_callback(
- Game& game, Request& r, DescriptionIndex, Worker* worker, PlayerImmovable& pi) {
- Warehouse* warehouse = static_cast(&pi);
-
- warehouse->get_portdock()->expedition_bootstrap()->handle_worker_callback(game, r, worker);
-}
-
-void ExpeditionBootstrap::handle_worker_callback(Game& game, Request& request, Worker* worker) {
- for (std::unique_ptr& ew : workers_) {
- if (ew->request.get() == &request) {
- ew->request.reset(nullptr); // deletes &request.
- ew->worker = worker;
-
- // This is not really correct. All the wares are not in the portdock,
- // so why should the worker be there. Well, it has to be somewhere
- // (location != nullptr) and it should be in our economy so he is
- // properly accounted for, so why not in the portdock. Alternatively
- // we could kill him and recreate him as soon as we bord the ship -
- // this should be no problem as workers are not gaining experience.
- worker->set_location(portdock_);
- worker->reset_tasks(game);
- worker->start_task_idle(game, 0, -1);
-
- is_ready(game);
- return;
- }
- }
- // Never here, otherwise we would have a callback for a request we know
- // nothing about.
- NEVER_HERE();
-}
-
void ExpeditionBootstrap::start() {
- assert(workers_.empty());
- assert(wares_.empty());
+ assert(queues_.empty());
// Load the buildcosts for the port building + builder
Warehouse* const warehouse = portdock_->get_warehouse();
@@ -123,18 +72,17 @@
// TODO(sirver): could try to get some of these directly form the warehouse.
// But this is really a premature optimization and should probably be
// handled in the economy code.
- wares_.resize(buildcost_size);
+ queues_.resize(buildcost_size + 1);
std::map::const_iterator it = buildcost.begin();
for (size_t i = 0; i < bu
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1191295-no-seafaring-builder into lp:widelands
Code LGTM so far - will have to look into the saveloading thing. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1191295-no-seafaring-builder/+merge/315525 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1191295-no-seafaring-builder into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
