From: Ingo Ruhnke <grumbel@gmail.com>
Date: Wed, 20 Aug 2014 01:27:50 +0000 (+0200)
Subject: Large scale refactor/rewrite of the AddonManager, adding cleaner separation between... 
X-Git-Url: https://git.verplant.org/?a=commitdiff_plain;h=0aab3bfe9a89f97ac42bfa614204e42c4cf1931f;p=supertux.git

Large scale refactor/rewrite of the AddonManager, adding cleaner separation between repository addons and installed ones
---

diff --git a/src/addon/addon.cpp b/src/addon/addon.cpp
index 44764ee05..ae9c63699 100644
--- a/src/addon/addon.cpp
+++ b/src/addon/addon.cpp
@@ -20,67 +20,72 @@
 #include <stdexcept>
 #include <sstream>
 
-#include "addon/md5.hpp"
 #include "lisp/parser.hpp"
 #include "util/reader.hpp"
 #include "util/writer.hpp"
 #include "util/log.hpp"
 
-std::string
-Addon::get_md5() const
+namespace {
+
+static const char* s_allowed_characters = "-0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
+
+Addon::Type addon_type_from_string(const std::string& type)
 {
-  if (!installed)
+  if (type == "world")
   {
-    if (stored_md5.empty())
-    {
-      log_warning << "Add-on not installed and no stored MD5 available" << std::endl;
-    }
-    return stored_md5;
+    return Addon::WORLD;
   }
-  else if (!calculated_md5.empty())
+  else if (type == "worldmap")
   {
-    return calculated_md5;
+    return Addon::WORLDMAP;
   }
-  else if (installed_physfs_filename.empty())
+  else if (type == "levelset")
   {
-    throw std::runtime_error("Tried to calculate MD5 of Add-on with unknown filename");
+    return Addon::LEVELSET;
   }
   else
   {
-    // TODO: this does not work as expected for some files -- IFileStream seems to not always behave like an ifstream.
-    //IFileStream ifs(installed_physfs_filename);
-    //std::string md5 = MD5(ifs).hex_digest();
-
-    MD5 md5;
-    PHYSFS_file* file;
-    file = PHYSFS_openRead(installed_physfs_filename.c_str());
-    unsigned char buffer[1024];
-    while (true) {
-      PHYSFS_sint64 len = PHYSFS_read(file, buffer, 1, sizeof(buffer));
-      if (len <= 0) break;
-      md5.update(buffer, len);
-    }
-    PHYSFS_close(file);
-
-    calculated_md5 = md5.hex_digest();
-    log_debug << "MD5 of " << title << ": " << calculated_md5 << std::endl;
-
-    return calculated_md5;
+    throw std::runtime_error("not a valid Addon::Type: " + type);
   }
 }
 
-void
+} // namespace
+
+std::unique_ptr<Addon>
 Addon::parse(const Reader& lisp)
 {
+  std::unique_ptr<Addon> addon(new Addon);
+
   try
   {
-    lisp.get("kind", kind);
-    lisp.get("title", title);
-    lisp.get("author", author);
-    lisp.get("license", license);
-    lisp.get("http-url", http_url);
-    lisp.get("file", suggested_filename);
-    lisp.get("md5", stored_md5);
+    if (!lisp.get("id", addon->m_id))
+    {
+      throw std::runtime_error("(id ...) field missing from addon description");
+    }
+
+    if (addon->m_id.empty())
+    {
+      throw std::runtime_error("addon id is empty");
+    }
+
+    if (addon->m_id.find_first_not_of(s_allowed_characters) != std::string::npos)
+    {
+      throw std::runtime_error("addon id contains illegal characters: " + addon->m_id);
+    }
+
+    lisp.get("version", addon->m_version);
+
+    std::string type;
+    lisp.get("type", type);
+    addon->m_type = addon_type_from_string(type);
+
+    lisp.get("title", addon->m_title);
+    lisp.get("author", addon->m_author);
+    lisp.get("license", addon->m_license);
+    lisp.get("http-url", addon->m_http_url);
+    lisp.get("md5", addon->m_md5);
+
+    return addon;
   }
   catch(const std::exception& err)
   {
@@ -90,7 +95,7 @@ Addon::parse(const Reader& lisp)
   }
 }
 
-void
+std::unique_ptr<Addon>
 Addon::parse(const std::string& fname)
 {
   try
@@ -98,8 +103,14 @@ Addon::parse(const std::string& fname)
     lisp::Parser parser;
     const lisp::Lisp* root = parser.parse(fname);
     const lisp::Lisp* addon = root->get_lisp("supertux-addoninfo");
-    if(!addon) throw std::runtime_error("file is not a supertux-addoninfo file.");
-    parse(*addon);
+    if(!addon)
+    {
+      throw std::runtime_error("file is not a supertux-addoninfo file.");
+    }
+    else
+    {
+      return parse(*addon);
+    }
   }
   catch(const std::exception& err)
   {
@@ -109,18 +120,54 @@ Addon::parse(const std::string& fname)
   }
 }
 
+Addon::Addon() :
+  m_id(),
+  m_version(0),
+  m_type(),
+  m_title(),
+  m_author(),
+  m_license(),
+  m_http_url(),
+  m_md5(),
+  m_install_filename(),
+  m_enabled(false)
+{}
+
+std::string
+Addon::get_filename() const
+{
+  return get_id() + ".zip";
+}
+
+std::string
+Addon::get_install_filename() const
+{
+  return m_install_filename;
+}
+
+bool
+Addon::is_installed() const
+{
+  return !m_install_filename.empty();
+}
+
 bool
-Addon::operator==(const Addon& addon2) const
+Addon::is_enabled() const
 {
-  std::string s1 = this->get_md5();
-  std::string s2 = addon2.get_md5();
+  return m_enabled;
+}
 
-  if ((s1 != "") && (s2 != "")) return (s1 == s2);
+void
+Addon::set_install_filename(const std::string& absolute_filename)
+{
+  m_install_filename = absolute_filename;
+}
 
-  if (this->title != addon2.title) return false;
-  if (this->author != addon2.author) return false;
-  if (this->kind != addon2.kind) return false;
-  return true;
+void
+Addon::set_enabled(bool v)
+{
+  m_enabled = v;
 }
 
+
 /* EOF */
diff --git a/src/addon/addon.hpp b/src/addon/addon.hpp
index 58314d0ca..66b3a16f9 100644
--- a/src/addon/addon.hpp
+++ b/src/addon/addon.hpp
@@ -17,74 +17,59 @@
 #ifndef HEADER_SUPERTUX_ADDON_ADDON_HPP
 #define HEADER_SUPERTUX_ADDON_ADDON_HPP
 
+#include <assert.h>
+#include <memory>
 #include <string>
 
 #include "util/reader_fwd.hpp"
 
-class AddonDescription
+class Addon
 {
 public:
-  std::string kind;
-  std::string title;
-  std::string author;
-  std::string license;
-  std::string http_url;
-  /** filename suggested by addon author, e.g. "pak0.zip" */
-  std::string suggested_filename;
-
-  AddonDescription() :
-    kind(),
-    title(),
-    author(),
-    license(),
-    http_url(),
-    suggested_filename()
-  {}
-};
+  static std::unique_ptr<Addon> parse(const Reader& lisp);
+  static std::unique_ptr<Addon> parse(const std::string& fname);
 
-/** Represents an (available or installed) Add-on, e.g. a level set */
-class Addon : public AddonDescription
-{
-public:
-  int id;
+  enum Type { WORLD, WORLDMAP, LEVELSET };
 
-  /** PhysFS filename on disk, e.g. "pak0.zip" */
-  std::string installed_physfs_filename;
+private:
+  // fields provided by the addon.zip itself
+  std::string m_id;
+  int m_version;
+  Type m_type;
+  std::string m_title;
+  std::string m_author;
+  std::string m_license;
+
+  // additional fields provided for addons from an addon repository
+  std::string m_http_url;
+  std::string m_md5;
+
+  // fields filled by the AddonManager
+  std::string m_install_filename;
+  bool m_enabled;
 
-  /** complete path and filename on disk, e.g. "/home/sommer/.supertux2/pak0.zip" */
-  std::string installed_absolute_filename;
+private:
+  Addon();
 
-  std::string stored_md5;
-  bool installed;
-  bool loaded;
+public:
+  std::string get_id() const { return m_id; }
 
-  /** Get MD5, based either on installed file's contents or stored value */
-  std::string get_md5() const;
+  Type get_type() const { return m_type; }
+  std::string get_title() const { return m_title; }
+  std::string get_author() const { return m_author; }
+  std::string get_license() const { return m_license; }
 
-  /** Read additional information from given contents of a (supertux-addoninfo ...) block */
-  void parse(const Reader& lisp);
+  std::string get_http_url() const { return m_http_url; }
+  std::string get_md5() const { return m_md5; }
 
-  /** Read additional information from given file */
-  void parse(const std::string& fname);
+  std::string get_filename() const;
+  std::string get_install_filename() const;
 
-  /** Checks if Add-on is the same as given one. If available, checks
-      MD5 sum, else relies on kind, author and title alone. */
-  bool operator==(const Addon& addon2) const;
+  bool is_installed() const;
+  bool is_enabled() const;
 
-public:
-  friend class AddonManager;
-
-  mutable std::string calculated_md5;
-
-  Addon(int id_) :
-    id(id_),
-    installed_physfs_filename(),
-    installed_absolute_filename(),
-    stored_md5(),
-    installed(),
-    loaded(),
-    calculated_md5()
-  {};
+  void set_install_filename(const std::string& absolute_filename);
+  void set_enabled(bool v);
 
 private:
   Addon(const Addon&) = delete;
diff --git a/src/addon/addon_list.cpp b/src/addon/addon_list.cpp
deleted file mode 100644
index 442b41a84..000000000
--- a/src/addon/addon_list.cpp
+++ /dev/null
@@ -1,96 +0,0 @@
-//  SuperTux
-//  Copyright (C) 2014 Ingo Ruhnke <grumbel@gmail.com>
-//
-//  This program is free software: you can redistribute it and/or modify
-//  it under the terms of the GNU General Public License as published by
-//  the Free Software Foundation, either version 3 of the License, or
-//  (at your option) any later version.
-//
-//  This program is distributed in the hope that it will be useful,
-//  but WITHOUT ANY WARRANTY; without even the implied warranty of
-//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-//  GNU General Public License for more details.
-//
-//  You should have received a copy of the GNU General Public License
-//  along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-#include "addon/addon_list.hpp"
-
-#include <memory>
-#include <sstream>
-#include <stdexcept>
-
-#include "addon/addon.hpp"
-#include "lisp/lisp.hpp"
-#include "lisp/list_iterator.hpp"
-#include "lisp/parser.hpp"
-#include "util/log.hpp"
-
-static const char* allowed_characters = "-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz";
-
-std::vector<std::unique_ptr<Addon> >
-AddonList::parse(const std::string& addoninfos)
-{
-  std::vector<std::unique_ptr<Addon> > m_addons;
-
-  try
-  {
-    lisp::Parser parser;
-    std::stringstream addoninfos_stream(addoninfos);
-    const lisp::Lisp* root = parser.parse(addoninfos_stream, "supertux-addons");
-
-    const lisp::Lisp* addons_lisp = root->get_lisp("supertux-addons");
-    if(!addons_lisp)
-    {
-      throw std::runtime_error("Downloaded file is not an Add-on list");
-    }
-
-    lisp::ListIterator iter(addons_lisp);
-    while(iter.next())
-    {
-      const std::string& token = iter.item();
-      if(token != "supertux-addoninfo")
-      {
-        log_warning << "Unknown token '" << token << "' in Add-on list" << std::endl;
-        continue;
-      }
-      std::unique_ptr<Addon> addon(new Addon(m_addons.size()));
-      addon->parse(*(iter.lisp()));
-      addon->installed = false;
-      addon->loaded = false;
-
-      // make sure the list of known Add-ons does not already contain this one
-      bool exists = false;
-      for (auto i = m_addons.begin(); i != m_addons.end(); ++i) {
-        if (**i == *addon) {
-          exists = true;
-          break;
-        }
-      }
-
-      if (exists)
-      {
-        // do nothing
-      }
-      else if (addon->suggested_filename.find_first_not_of(allowed_characters) != std::string::npos)
-      {
-        // make sure the Add-on's file name does not contain weird characters
-        log_warning << "Add-on \"" << addon->title << "\" contains unsafe file name. Skipping." << std::endl;
-      }
-      else
-      {
-        m_addons.push_back(std::move(addon));
-      }
-    }
-
-    return m_addons;
-  }
-  catch(std::exception& e)
-  {
-    std::stringstream msg;
-    msg << "Problem when reading Add-on list: " << e.what();
-    throw std::runtime_error(msg.str());
-  }
-}
-
-/* EOF */
diff --git a/src/addon/addon_list.hpp b/src/addon/addon_list.hpp
deleted file mode 100644
index 723382b5e..000000000
--- a/src/addon/addon_list.hpp
+++ /dev/null
@@ -1,43 +0,0 @@
-//  SuperTux
-//  Copyright (C) 2014 Ingo Ruhnke <grumbel@gmail.com>
-//
-//  This program is free software: you can redistribute it and/or modify
-//  it under the terms of the GNU General Public License as published by
-//  the Free Software Foundation, either version 3 of the License, or
-//  (at your option) any later version.
-//
-//  This program is distributed in the hope that it will be useful,
-//  but WITHOUT ANY WARRANTY; without even the implied warranty of
-//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-//  GNU General Public License for more details.
-//
-//  You should have received a copy of the GNU General Public License
-//  along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-#ifndef HEADER_SUPERTUX_ADDON_ADDON_LIST_HPP
-#define HEADER_SUPERTUX_ADDON_ADDON_LIST_HPP
-
-#include <string>
-#include <vector>
-#include <memory>
-
-#include "addon/addon.hpp"
-
-class AddonList
-{
-private:
-public:
-  static std::vector<std::unique_ptr<Addon> > parse(const std::string& str);
-
-public:
-  AddonList();
-
-
-private:
-  AddonList(const AddonList&) = delete;
-  AddonList& operator=(const AddonList&) = delete;
-};
-
-#endif
-
-/* EOF */
diff --git a/src/addon/addon_manager.cpp b/src/addon/addon_manager.cpp
index 77b72fe2b..49ce4e42e 100644
--- a/src/addon/addon_manager.cpp
+++ b/src/addon/addon_manager.cpp
@@ -1,5 +1,6 @@
 //  SuperTux - Add-on Manager
 //  Copyright (C) 2007 Christoph Sommer <christoph.sommer@2007.expires.deltadevelopment.de>
+//                2014 Ingo Ruhnke <grumbel@gmail.com>
 //
 //  This program is free software: you can redistribute it and/or modify
 //  it under the terms of the GNU General Public License as published by
@@ -18,17 +19,18 @@
 
 #include <config.h>
 #include <version.h>
-#include <iostream>
 
 #include <algorithm>
+#include <iostream>
 #include <memory>
 #include <physfs.h>
 #include <sstream>
 #include <stdexcept>
+#include <stdio.h>
 #include <sys/stat.h>
 
 #include "addon/addon.hpp"
-#include "addon/addon_list.hpp"
+#include "addon/md5.hpp"
 #include "lisp/list_iterator.hpp"
 #include "lisp/parser.hpp"
 #include "util/file_system.hpp"
@@ -38,18 +40,58 @@
 
 namespace {
 
-const char* allowed_characters = "-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz";
+MD5 md5_from_file(const std::string& filename)
+{
+  // TODO: this does not work as expected for some files -- IFileStream seems to not always behave like an ifstream.
+  //IFileStream ifs(installed_physfs_filename);
+  //std::string md5 = MD5(ifs).hex_digest();
 
-} // namespace
+  MD5 md5;
+
+  unsigned char buffer[1024];
+  PHYSFS_file* file = PHYSFS_openRead(filename.c_str());
+  while (true)
+  {
+    PHYSFS_sint64 len = PHYSFS_read(file, buffer, 1, sizeof(buffer));
+    if (len <= 0) break;
+    md5.update(buffer, len);
+  }
+  PHYSFS_close(file);
+
+  return md5;
+}
 
+bool has_suffix(const std::string& str, const std::string& suffix)
+{
+  if (str.length() >= suffix.length())
+    return str.compare(str.length() - suffix.length(), suffix.length(), suffix) == 0;
+  else
+    return false;
+}
+
+} // namespace
 
 AddonManager::AddonManager(const std::string& addon_directory,
-                           std::vector<std::string>& ignored_addon_filenames) :
+                           std::vector<std::string>& ignored_addon_ids) :
   m_downloader(),
   m_addon_directory(addon_directory),
-  m_addons(),
-  m_ignored_addon_filenames(ignored_addon_filenames)
+  //m_repository_url("http://addons.supertux.googlecode.com/git/index-0_3_5.nfo"),
+  m_repository_url("http://localhost:8000/index-0_4_0.nfo"),
+  m_ignored_addon_ids(ignored_addon_ids),
+  m_installed_addons(),
+  m_repository_addons()
 {
+  PHYSFS_mkdir(m_addon_directory.c_str());
+
+  add_installed_addons();
+  for(auto& addon : m_installed_addons)
+  {
+    if (std::find(m_ignored_addon_ids.begin(), m_ignored_addon_ids.end(),
+                  addon->get_id()) != m_ignored_addon_ids.end())
+    {
+      enable_addon(addon->get_id());
+    }
+  }
 }
 
 AddonManager::~AddonManager()
@@ -57,302 +99,337 @@ AddonManager::~AddonManager()
 }
 
 Addon&
-AddonManager::get_addon(int id)
+AddonManager::get_repository_addon(const AddonId& id)
 {
-  if (0 <= id && id < static_cast<int>(m_addons.size()))
+  auto it = std::find_if(m_repository_addons.begin(), m_repository_addons.end(),
+                         [&id](const std::unique_ptr<Addon>& addon)
+                         {
+                           return addon->get_id() == id;
+                         });
+
+  if (it != m_repository_addons.end())
   {
-    return *m_addons[id];
+    return **it;
   }
   else
   {
-    throw std::runtime_error("AddonManager::get_addon(): id out of range: " + std::to_string(id));
+    throw std::runtime_error("Couldn't find repository Addon with id: " + id);
   }
 }
 
-const std::vector<std::unique_ptr<Addon> >&
-AddonManager::get_addons() const
+Addon&
+AddonManager::get_installed_addon(const AddonId& id)
 {
-  /*
-    for (std::vector<Addon>::iterator it = installed_addons.begin(); it != installed_addons.end(); ++it) {
-    Addon& addon = *it;
-    if (addon.md5.empty()) addon.md5 = calculate_md5(addon);
-    }
-  */
-  return m_addons;
+  auto it = std::find_if(m_installed_addons.begin(), m_installed_addons.end(),
+                         [&id](const std::unique_ptr<Addon>& addon)
+                         {
+                           return addon->get_id() == id;
+                         });
+
+  if (it != m_installed_addons.end())
+  {
+    return **it;
+  }
+  else
+  {
+    throw std::runtime_error("Couldn't find installed Addon with id: " + id);
+  }
+}
+
+std::vector<AddonId>
+AddonManager::get_repository_addons() const
+{
+  std::vector<AddonId> results;
+  results.reserve(m_repository_addons.size());
+  std::transform(m_repository_addons.begin(), m_repository_addons.end(),
+                 std::back_inserter(results),
+                 [](const std::unique_ptr<Addon>& addon)
+                 {
+                   return addon->get_id();
+                 });
+  return results;
+}
+
+
+std::vector<AddonId>
+AddonManager::get_installed_addons() const
+{
+  std::vector<AddonId> results;
+  results.reserve(m_installed_addons.size());
+  std::transform(m_installed_addons.begin(), m_installed_addons.end(),
+                 std::back_inserter(results),
+                 [](const std::unique_ptr<Addon>& addon)
+                 {
+                   return addon->get_id();
+                 });
+  return results;
 }
 
 bool
 AddonManager::has_online_support() const
 {
-#ifdef HAVE_LIBCURL
   return true;
-#else
-  return false;
-#endif
 }
 
 void
 AddonManager::check_online()
 {
-  const char* baseUrl = "http://addons.supertux.googlecode.com/git/index-0_3_5.nfo";
-  std::string addoninfos = m_downloader.download(baseUrl);
-
-  AddonList::parse(addoninfos);
+  std::string addoninfos = m_downloader.download(m_repository_url);
+  m_repository_addons = parse_addon_infos(addoninfos);
 }
 
 void
-AddonManager::install(Addon& addon)
+AddonManager::install_addon(const AddonId& addon_id)
 {
-  if (addon.installed)
-  {
-    throw std::runtime_error("Tried installing installed Add-on");
-  }
+  log_debug << "installing addon " << addon_id << std::endl;
+  Addon& repository_addon = get_repository_addon(addon_id);
 
-  // make sure the Add-on's file name does not contain weird characters
-  if (addon.suggested_filename.find_first_not_of(allowed_characters) != std::string::npos)
-  {
-    throw std::runtime_error("Add-on has unsafe file name (\""+addon.suggested_filename+"\")");
-  }
+  std::string install_filename = FileSystem::join(m_addon_directory, repository_addon.get_filename());
 
-  std::string filename = FileSystem::join(m_addon_directory, addon.suggested_filename);
+  m_downloader.download(repository_addon.get_http_url(), install_filename);
 
-  // make sure its file doesn't already exist
-  if (PHYSFS_exists(filename.c_str()))
+  MD5 md5 = md5_from_file(install_filename);
+  if (repository_addon.get_md5() != md5.hex_digest())
   {
-    filename = FileSystem::join(m_addon_directory, addon.stored_md5 + "_" + addon.suggested_filename);
-    if (PHYSFS_exists(filename.c_str()))
+    if (PHYSFS_delete(install_filename.c_str()) == 0)
     {
-      throw std::runtime_error("Add-on of suggested filename already exists (\"" +
-                               addon.suggested_filename + "\", \"" + filename + "\")");
+      log_warning << "PHYSFS_delete failed: " << PHYSFS_getLastError() << std::endl;
     }
-  }
-
-  m_downloader.download(addon.http_url, filename);
-
-  addon.installed = true;
-  addon.installed_physfs_filename = filename;
-  std::string writeDir = PHYSFS_getWriteDir();
-  addon.installed_absolute_filename = FileSystem::join(writeDir, filename);
-  addon.loaded = false;
 
-  if (addon.get_md5() != addon.stored_md5)
-  {
-    addon.installed = false;
-    PHYSFS_delete(filename.c_str());
-    std::string why = "MD5 checksums differ";
-    throw std::runtime_error("Downloading Add-on failed: " + why);
-  }
-
-  log_debug << "Finished downloading \"" << addon.installed_absolute_filename << "\". Enabling Add-on." << std::endl;
-
-  enable(addon);
-}
-
-void
-AddonManager::remove(Addon& addon)
-{
-  if (!addon.installed)
-  {
-    throw std::runtime_error("Tried removing non-installed Add-on");
-  }
-  else if (addon.installed_physfs_filename.find_first_not_of(allowed_characters) != std::string::npos)
-  {
-    // make sure the Add-on's file name does not contain weird characters
-    throw std::runtime_error("Add-on has unsafe file name (\""+addon.installed_physfs_filename+"\")");
+    throw std::runtime_error("Downloading Add-on failed: MD5 checksums differ");
   }
   else
   {
-    unload(addon);
-
-    log_debug << "deleting file \"" << addon.installed_absolute_filename << "\"" << std::endl;
-    PHYSFS_delete(addon.installed_absolute_filename.c_str());
-    addon.installed = false;
-
-    // FIXME: As we don't know anything more about it (e.g. where to get it), remove it from list of known Add-ons
-  }
-}
-
-void
-AddonManager::disable(Addon& addon)
-{
-  unload(addon);
-
-  std::string filename = addon.installed_physfs_filename;
-  if (std::find(m_ignored_addon_filenames.begin(), m_ignored_addon_filenames.end(),
-                filename) == m_ignored_addon_filenames.end())
-  {
-    m_ignored_addon_filenames.push_back(filename);
+    const char* realdir = PHYSFS_getRealDir(install_filename.c_str());
+    if (!realdir)
+    {
+      throw std::runtime_error("PHYSFS_getRealDir failed: " + install_filename);
+    }
+    else
+    {
+      add_installed_archive(install_filename);
+    }
   }
 }
 
 void
-AddonManager::enable(Addon& addon)
+AddonManager::uninstall_addon(const AddonId& addon_id)
 {
-  load(addon);
-
-  std::string filename = addon.installed_physfs_filename;
-  auto it = std::find(m_ignored_addon_filenames.begin(), m_ignored_addon_filenames.end(), filename);
-  if (it != m_ignored_addon_filenames.end())
+  log_debug << "uninstalling addon " << addon_id << std::endl;
+  Addon& addon = get_installed_addon(addon_id);
+  if (addon.is_enabled())
   {
-    m_ignored_addon_filenames.erase(it);
+    disable_addon(addon_id);
   }
+  log_debug << "deleting file \"" << addon.get_install_filename() << "\"" << std::endl;
+  PHYSFS_delete(addon.get_install_filename().c_str());
+  m_installed_addons.erase(std::remove_if(m_installed_addons.begin(), m_installed_addons.end(),
+                                          [&addon](const std::unique_ptr<Addon>& rhs)
+                                          {
+                                            return addon.get_id() == rhs->get_id();
+                                          }),
+                           m_installed_addons.end());
 }
 
 void
-AddonManager::unload(Addon& addon)
+AddonManager::enable_addon(const AddonId& addon_id)
 {
-  if (!addon.installed)
-  {
-    throw std::runtime_error("Tried unloading non-installed Add-on");
-  }
-  else if (!addon.loaded)
+  log_debug << "enabling addon " << addon_id << std::endl;
+  Addon& addon = get_installed_addon(addon_id);
+  if (addon.is_enabled())
   {
-    // do nothing
+    log_warning << "Tried enabling already enabled Add-on" << std::endl;
   }
   else
   {
-    log_debug << "Removing archive \"" << addon.installed_absolute_filename << "\" from search path" << std::endl;
-    if (PHYSFS_removeFromSearchPath(addon.installed_absolute_filename.c_str()) == 0) {
-      log_warning << "Could not remove " << addon.installed_absolute_filename << " from search path. Ignoring." << std::endl;
-      return;
+    log_debug << "Adding archive \"" << addon.get_install_filename() << "\" to search path" << std::endl;
+    //int PHYSFS_mount(addon.installed_install_filename.c_str(), "addons/", 0)
+    if (PHYSFS_addToSearchPath(addon.get_install_filename().c_str(), 0) == 0)
+    {
+      log_warning << "Could not add " << addon.get_install_filename() << " to search path: "
+                  << PHYSFS_getLastError() << std::endl;
+    }
+    else
+    {
+      addon.set_enabled(true);
     }
-
-    addon.loaded = false;
   }
 }
 
 void
-AddonManager::load(Addon& addon)
+AddonManager::disable_addon(const AddonId& addon_id)
 {
-  if (!addon.installed)
+  log_debug << "disabling addon " << addon_id << std::endl;
+  Addon& addon = get_installed_addon(addon_id);
+  if (!addon.is_enabled())
   {
-    throw std::runtime_error("Tried loading non-installed Add-on");
-  }
-  else if (addon.loaded)
-  {
-    // do nothing
+    log_warning << "Tried disabling already disabled Add-On" << std::endl;
   }
   else
   {
-    log_debug << "Adding archive \"" << addon.installed_absolute_filename << "\" to search path" << std::endl;
-    if (PHYSFS_addToSearchPath(addon.installed_absolute_filename.c_str(), 0) == 0) {
-      log_warning << "Could not add " << addon.installed_absolute_filename << " to search path. Ignoring." << std::endl;
-      return;
+    log_debug << "Removing archive \"" << addon.get_install_filename() << "\" from search path" << std::endl;
+    if (PHYSFS_removeFromSearchPath(addon.get_install_filename().c_str()) == 0)
+    {
+      log_warning << "Could not remove " << addon.get_install_filename() << " from search path: "
+                  << PHYSFS_getLastError() << std::endl;
+    }
+    else
+    {
+      addon.set_enabled(false);
     }
-
-    addon.loaded = true;
   }
 }
 
-void
-AddonManager::load_addons()
+std::vector<std::string>
+AddonManager::scan_for_archives() const
 {
-  PHYSFS_mkdir(m_addon_directory.c_str());
+  std::vector<std::string> archives;
 
-  // unload all Addons and forget about them
-  for (auto& addon : m_addons)
+  // Search for archives and add them to the search path
+  std::unique_ptr<char*, decltype(&PHYSFS_freeList)>
+    rc(PHYSFS_enumerateFiles(m_addon_directory.c_str()),
+       PHYSFS_freeList);
+  for(char** i = rc.get(); *i != 0; ++i)
   {
-    if (addon->installed && addon->loaded)
+    if (has_suffix(*i, ".zip"))
     {
-      unload(*addon);
+      std::string archive = FileSystem::join(m_addon_directory, *i);
+      if (PHYSFS_exists(archive.c_str()))
+      {
+        archives.push_back(archive);
+      }
     }
   }
-  m_addons.clear();
 
-  // Search for archives and add them to the search path
-  char** rc = PHYSFS_enumerateFiles(m_addon_directory.c_str());
+  return archives;
+}
 
-  for(char** i = rc; *i != 0; ++i)
+std::string
+AddonManager::scan_for_info(const std::string& archive_os_path) const
+{
+  std::unique_ptr<char*, decltype(&PHYSFS_freeList)>
+    rc2(PHYSFS_enumerateFiles("/"),
+        PHYSFS_freeList);
+  for(char** j = rc2.get(); *j != 0; ++j)
   {
-    // get filename of potential archive
-    std::string filename = *i;
+    log_debug << "enumerating: " << std::string(*j) << std::endl;
+    if (has_suffix(*j, ".nfo"))
+    {
+      std::string nfo_filename = FileSystem::join("/", *j);
+
+      // make sure it's in the current archive_os_path
+      const char* realdir = PHYSFS_getRealDir(nfo_filename.c_str());
+      if (!realdir)
+      {
+        log_warning << "PHYSFS_getRealDir() failed for " << nfo_filename << ": " << PHYSFS_getLastError() << std::endl;
+      }
+      else
+      {
+        log_debug << "compare: " << realdir << " " << archive_os_path << std::endl;
+        if (realdir == archive_os_path)
+        {
+          return nfo_filename;
+        }
+      }
+    }
+  }
 
-    std::cout << m_addon_directory << " -> " << filename << std::endl;
+  return std::string();
+}
 
-    const std::string archiveDir = PHYSFS_getRealDir(filename.c_str());
-    std::string fullFilename = FileSystem::join(archiveDir, filename);
+void
+AddonManager::add_installed_archive(const std::string& archive)
+{
+  const char* realdir = PHYSFS_getRealDir(archive.c_str());
+  if (!realdir)
+  {
+    log_warning << "PHYSFS_getRealDir() failed for " << archive << ": "
+                << PHYSFS_getLastError() << std::endl;
+  }
+  else
+  {
+    std::string os_path = FileSystem::join(realdir, archive);
 
-    /*
-    // make sure it's in the writeDir
-    std::string writeDir = PHYSFS_getWriteDir();
-    if (filename.compare(0, writeDir.length(), writeDir) != 0) continue;
-    */
+    PHYSFS_addToSearchPath(os_path.c_str(), 0);
 
-    // make sure it looks like an archive
-    std::string archiveExt = ".zip";
-    if (fullFilename.compare(fullFilename.length() - archiveExt.length(),
-                             archiveExt.length(), archiveExt) != 0)
+    std::string nfo_filename = scan_for_info(os_path);
+
+    if (nfo_filename.empty())
+    {
+      log_warning << "Couldn't find .nfo file for " << os_path << std::endl;
+    }
+    else
     {
-      continue;
+      try
+      {
+        std::unique_ptr<Addon> addon = Addon::parse(nfo_filename);
+        addon->set_install_filename(os_path);
+        m_installed_addons.push_back(std::move(addon));
+      }
+      catch (const std::runtime_error& e)
+      {
+        log_warning << "Could not load add-on info for " << archive << ": " << e.what() << std::endl;
+      }
     }
 
-    // make sure it exists
-    struct stat stats;
-    if (stat(fullFilename.c_str(), &stats) != 0) continue;
+    PHYSFS_removeFromSearchPath(os_path.c_str());
+  }
+}
 
-    // make sure it's an actual file
-    if (!S_ISREG(stats.st_mode)) continue;
+void
+AddonManager::add_installed_addons()
+{
+  auto archives = scan_for_archives();
 
-    log_debug << "Found archive \"" << fullFilename << "\"" << std::endl;
+  for(auto archive : archives)
+  {
+    add_installed_archive(archive);
+  }
+}
 
-    // add archive to search path
-    PHYSFS_addToSearchPath(fullFilename.c_str(), 0);
+AddonManager::AddonList
+AddonManager::parse_addon_infos(const std::string& addoninfos) const
+{
+  AddonList m_addons;
 
-    // Search for infoFiles
-    std::string infoFileName = "";
-    char** rc2 = PHYSFS_enumerateFiles(m_addon_directory.c_str());
-    for(char** j = rc2; *j != 0; ++j)
+  try
+  {
+    lisp::Parser parser;
+    std::stringstream addoninfos_stream(addoninfos);
+    const lisp::Lisp* root = parser.parse(addoninfos_stream, "supertux-addons");
+    const lisp::Lisp* addons_lisp = root->get_lisp("supertux-addons");
+    if(!addons_lisp)
     {
-      // get filename of potential infoFile
-      std::string potentialInfoFileName = *j;
-
-      // make sure it looks like an infoFile
-      static const std::string infoExt = ".nfo";
-      if (potentialInfoFileName.length() <= infoExt.length())
-        continue;
-
-      if (potentialInfoFileName.compare(potentialInfoFileName.length()-infoExt.length(), infoExt.length(), infoExt) != 0)
-        continue;
-
-      // make sure it's in the current archive
-      std::string infoFileDir = PHYSFS_getRealDir(potentialInfoFileName.c_str());
-      if (infoFileDir == fullFilename)
-      {
-        // found infoFileName
-        infoFileName = potentialInfoFileName;
-        break;
-      }
+      throw std::runtime_error("Downloaded file is not an Add-on list");
     }
-    PHYSFS_freeList(rc2);
-
-    // if we have an infoFile, it's an Addon
-    if (!infoFileName.empty())
+    else
     {
-      try
+      lisp::ListIterator iter(addons_lisp);
+      while(iter.next())
       {
-        std::unique_ptr<Addon> addon(new Addon(m_addons.size()));
-        addon->parse(infoFileName);
-        addon->installed = true;
-        addon->installed_physfs_filename = filename;
-        addon->installed_absolute_filename = fullFilename;
-        addon->loaded = true;
-
-        // check if the Addon is disabled
-        if (std::find(m_ignored_addon_filenames.begin(), m_ignored_addon_filenames.end(), filename) != m_ignored_addon_filenames.end())
+        const std::string& token = iter.item();
+        if(token != "supertux-addoninfo")
         {
-          unload(*addon);
+          log_warning << "Unknown token '" << token << "' in Add-on list" << std::endl;
+        }
+        else
+        {
+          std::unique_ptr<Addon> addon = Addon::parse(*iter.lisp());
+          m_addons.push_back(std::move(addon));
         }
-
-        m_addons.push_back(std::move(addon));
-      }
-      catch (const std::runtime_error& e)
-      {
-        log_warning << "Could not load add-on info for " << fullFilename << ", loading as unmanaged:" << e.what() << std::endl;
       }
+
+      return m_addons;
     }
   }
+  catch(const std::exception& e)
+  {
+    std::stringstream msg;
+    msg << "Problem when reading Add-on list: " << e.what();
+    throw std::runtime_error(msg.str());
+  }
 
-  PHYSFS_freeList(rc);
+  return m_addons;
 }
 
 /* EOF */
diff --git a/src/addon/addon_manager.hpp b/src/addon/addon_manager.hpp
index 57ae3de9e..cd1167165 100644
--- a/src/addon/addon_manager.hpp
+++ b/src/addon/addon_manager.hpp
@@ -1,5 +1,6 @@
 //  SuperTux - Add-on Manager
 //  Copyright (C) 2007 Christoph Sommer <christoph.sommer@2007.expires.deltadevelopment.de>
+//                2014 Ingo Ruhnke <grumbel@gmail.com>
 //
 //  This program is free software: you can redistribute it and/or modify
 //  it under the terms of the GNU General Public License as published by
@@ -27,53 +28,56 @@
 #include "util/writer_fwd.hpp"
 
 class Addon;
+class AddonRepository;
 
-typedef int AddonId;
+typedef std::string AddonId;
 
 /** Checks for, installs and removes Add-ons */
 class AddonManager : public Currenton<AddonManager>
 {
+private:
+  typedef std::vector<std::unique_ptr<Addon> > AddonList;
+
+  Downloader m_downloader;
+  std::string m_addon_directory;
+  std::string m_repository_url;
+  std::vector<std::string>& m_ignored_addon_ids;
+
+  AddonList m_installed_addons;
+  AddonList m_repository_addons;
+
 public:
   AddonManager(const std::string& addon_directory,
-               std::vector<std::string>& ignored_addon_filenames_);
+               std::vector<std::string>& enabled_addons_);
   ~AddonManager();
 
-  /** returns a list of installed Add-ons */
-  const std::vector<std::unique_ptr<Addon> >& get_addons() const;
-
-  /** Returns true if online support is available */
   bool has_online_support() const;
-
-  /** downloads list of available Add-ons */
   void check_online();
 
-  /** Download and install Add-on */
-  void install(Addon& addon);
-  /** Physically delete Add-on */
-  void remove(Addon& addon);
+  std::vector<AddonId> get_repository_addons() const;
+  std::vector<AddonId> get_installed_addons() const;
 
-  /** Load Add-on and mark as to be loaded automatically */
-  void enable(Addon& addon);
-  /** Unload Add-on and mark as not to be loaded automatically */
-  void disable(Addon& addon);
+  Addon& get_repository_addon(const AddonId& addon);
+  Addon& get_installed_addon(const AddonId& addon);
 
-  Addon& get_addon(int id);
-  int get_num_addons() const { return static_cast<int>(m_addons.size()); }
+  void install_addon(const AddonId& addon_id);
+  void uninstall_addon(const AddonId& addon_id);
 
-  /** Loads all enabled Add-ons, i.e. adds them to the search path */
-  void load_addons();
+  void enable_addon(const AddonId& addon_id);
+  void disable_addon(const AddonId& addon_id);
 
 private:
-  /** Add Add-on to search path */
-  void load(Addon& addon);
-  /** Remove Add-on from search path */
-  void unload(Addon& addon);
+  std::vector<std::string> scan_for_archives() const;
+  void add_installed_addons();
+  AddonList parse_addon_infos(const std::string& addoninfos) const;
 
-private:
-  Downloader m_downloader;
-  std::string m_addon_directory;
-  std::vector<std::unique_ptr<Addon> > m_addons;
-  std::vector<std::string>& m_ignored_addon_filenames;
+  /** add \a archive, given as physfs path, to the list of installed
+      archives */
+  void add_installed_archive(const std::string& archive);
+
+  /** search for an .nfo file in the top level directory that
+      originates from \a archive, \a archive is a OS path */
+  std::string scan_for_info(const std::string& archive) const;
 
 private:
   AddonManager(const AddonManager&) = delete;
diff --git a/src/addon/downloader.cpp b/src/addon/downloader.cpp
index 3ca538875..7f795a54f 100644
--- a/src/addon/downloader.cpp
+++ b/src/addon/downloader.cpp
@@ -61,6 +61,8 @@ Downloader::download(const std::string& url,
                      size_t (*write_func)(void* ptr, size_t size, size_t nmemb, void* userdata),
                      void* userdata)
 {
+  log_info << "Downloading " << url << std::endl;
+
   char error_buffer[CURL_ERROR_SIZE+1];
 
   CURL* curl_handle = curl_easy_init();
diff --git a/src/supertux/main.cpp b/src/supertux/main.cpp
index ce0532015..d04eb5fb3 100644
--- a/src/supertux/main.cpp
+++ b/src/supertux/main.cpp
@@ -327,7 +327,6 @@ Main::launch_game()
 
   timelog("addons");
   AddonManager addon_manager("addons", g_config->disabled_addon_filenames);
-  addon_manager.load_addons();
 
   timelog(0);
 
diff --git a/src/supertux/menu/addon_menu.cpp b/src/supertux/menu/addon_menu.cpp
index 43ed81c97..615305d7d 100644
--- a/src/supertux/menu/addon_menu.cpp
+++ b/src/supertux/menu/addon_menu.cpp
@@ -26,8 +26,61 @@
 #include "gui/menu_item.hpp"
 #include "util/gettext.hpp"
 
+namespace {
+
+#define IS_REPOSITORY_MENU_ID(idx) ((idx - MNID_ADDON_LIST_START) % 2 == 0)
+#define IS_INSTALLED_MENU_ID(idx) ((idx - MNID_ADDON_LIST_START) % 2 == 1)
+
+#define MAKE_REPOSITORY_MENU_ID(idx) (MNID_ADDON_LIST_START + 2*idx+0)
+#define MAKE_INSTALLED_MENU_ID(idx) (MNID_ADDON_LIST_START + 2*idx+1)
+
+#define UNPACK_REPOSITORY_MENU_ID(idx) (((idx - MNID_ADDON_LIST_START) - 0) / 2)
+#define UNPACK_INSTALLED_MENU_ID(idx) (((idx - MNID_ADDON_LIST_START) - 1) / 2)
+
+std::string addon_type_to_translated_string(Addon::Type type)
+{
+  switch (type)
+  {
+    case Addon::LEVELSET:
+      return _("Levelset");
+
+    case Addon::WORLDMAP:
+      return _("Worldmap");
+
+    case Addon::WORLD:
+      return _("World");
+
+    default:
+      return _("Unknown");
+  }
+}
+
+std::string generate_menu_item_text(const Addon& addon)
+{
+  std::string text;
+  std::string type = addon_type_to_translated_string(addon.get_type());
+
+  if(!addon.get_author().empty())
+  {
+    text = str(boost::format(_("%s \"%s\" by \"%s\""))
+               % type % addon.get_title() % addon.get_author());
+  }
+  else
+  {
+    // Only addon type and name, no need for translation.
+    text = str(boost::format("%s \"%s\"")
+               % type % addon.get_title());
+  }
+
+  return text;
+}
+
+} // namespace
+
 AddonMenu::AddonMenu() :
-  m_addon_manager(*AddonManager::current())
+  m_addon_manager(*AddonManager::current()),
+  m_installed_addons(),
+  m_repository_addons()
 {
   refresh();
 }
@@ -35,26 +88,42 @@ AddonMenu::AddonMenu() :
 void
 AddonMenu::refresh()
 {
-  clear();
-
-  // refresh list of addons
-  const auto& addons_ref = m_addon_manager.get_addons();
-  std::vector<std::reference_wrapper<Addon> > addons;
-  std::transform(addons_ref.begin(), addons_ref.end(), std::back_inserter(addons),
-                 [](const std::unique_ptr<Addon>& addon) -> Addon& {
-                   return *addon.get();
-                 });
+  m_installed_addons = m_addon_manager.get_installed_addons();
+  m_repository_addons = m_addon_manager.get_repository_addons();
 
-  // sort list
-  std::sort(addons.begin(), addons.end(),
+#ifdef GRUMBEL
+  std::sort(m_addons.begin(), m_addons.end(),
             [](const Addon& lhs, const Addon& rhs)
             {
               return lhs.title < lhs.title;
             });
+#endif
 
+  rebuild_menu();
+}
+
+void
+AddonMenu::rebuild_menu()
+{
+  clear();
   add_label(_("Add-ons"));
   add_hl();
 
+  
+  if (!m_installed_addons.empty())
+  {
+    int idx = 0;
+    for (const auto& addon_id : m_installed_addons)
+    {
+      const Addon& addon = m_addon_manager.get_installed_addon(addon_id);
+      std::string text = generate_menu_item_text(addon);
+      add_toggle(MAKE_INSTALLED_MENU_ID(idx), text, addon.is_enabled());   
+      idx += 1;
+    }
+
+    add_hl();
+  }
+
   if (!m_addon_manager.has_online_support())
   {
     add_inactive(MNID_CHECK_ONLINE, std::string(_("Check Online (disabled)")));
@@ -64,55 +133,15 @@ AddonMenu::refresh()
     add_entry(MNID_CHECK_ONLINE, std::string(_("Check Online")));
   }
 
-  //add_hl();
-
-  for (auto& addon_ : addons)
   {
-    Addon& addon = addon_.get();
-    std::string text = "";
-
-    if (!addon.kind.empty())
-    {
-      std::string kind = addon.kind;
-      if(addon.kind == "Levelset") {
-        kind = _("Levelset");
-      }
-      else if(addon.kind == "Worldmap") {
-        kind = _("Worldmap");
-      }
-      else if(addon.kind == "World") {
-        kind = _("World");
-      }
-      else if(addon.kind == "Level") {
-        kind = _("Level");
-      }
-
-      if(!addon.author.empty())
-      {
-        text = str(boost::format(_("%s \"%s\" by \"%s\""))
-                   % kind % addon.title % addon.author);
-      }
-      else
-      {
-        // Only addon type and name, no need for translation.
-        text = str(boost::format("%s \"%s\"")
-                   % kind % addon.title);
-      }
-    }
-    else
+    int idx = 0;
+    for (const auto& addon_id : m_repository_addons)
     {
-      if (!addon.author.empty())
-      {
-        text = str(boost::format(_("\"%s\" by \"%s\""))
-                   % addon.title % addon.author);
-      }
-      else {
-        // Only addon name, no need for translation.
-        text = str(boost::format("\"%s\"")
-                   % addon.title);
-      }
+      const Addon& addon = m_addon_manager.get_repository_addon(addon_id);
+      std::string text = generate_menu_item_text(addon);
+      add_entry(MAKE_REPOSITORY_MENU_ID(idx), "Install " + text);
+      idx += 1;
     }
-    add_toggle(MNID_ADDON_LIST_START + addon.id, text, addon.loaded);
   }
 
   add_hl();
@@ -128,54 +157,55 @@ AddonMenu::menu_action(MenuItem* item)
     {
       m_addon_manager.check_online();
       refresh();
-      set_active_item(item->id);
     }
     catch (std::exception& e)
     {
       log_warning << "Check for available Add-ons failed: " << e.what() << std::endl;
     }
   }
-  else if ((MNID_ADDON_LIST_START <= item->id) && (item->id < MNID_ADDON_LIST_START + m_addon_manager.get_num_addons()))
+  else if (MNID_ADDON_LIST_START <= item->id)
   {
-    int addon_id = item->id - MNID_ADDON_LIST_START;
-    Addon& addon = m_addon_manager.get_addon(addon_id);
-    if (!addon.installed)
-    {
-      try
-      {
-        m_addon_manager.install(addon);
-      }
-      catch (std::exception& e)
-      {
-        log_warning << "Installing Add-on failed: " << e.what() << std::endl;
-      }
-      set_toggled(item->id, addon.loaded);
-    }
-    else if (!addon.loaded)
+    if (IS_INSTALLED_MENU_ID(item->id))
     {
-      try
-      {
-        m_addon_manager.enable(addon);
-      }
-      catch (std::exception& e)
+      int idx = UNPACK_INSTALLED_MENU_ID(item->id);
+      if (0 <= idx && idx < static_cast<int>(m_installed_addons.size()))
       {
-        log_warning << "Enabling Add-on failed: " << e.what() << std::endl;
+        const Addon& addon = m_addon_manager.get_installed_addon(m_installed_addons[idx]);
+        if(addon.is_enabled())
+        {
+          m_addon_manager.enable_addon(addon.get_id());
+          set_toggled(item->id, addon.is_enabled());
+        }
+        else
+        {
+          m_addon_manager.enable_addon(addon.get_id());
+          set_toggled(item->id, addon.is_enabled());
+        }
       }
-      set_toggled(item->id, addon.loaded);
     }
-    else
+    else if (IS_REPOSITORY_MENU_ID(item->id))
     {
-      try
-      {
-        m_addon_manager.disable(addon);
-      }
-      catch (std::exception& e)
+      int idx = UNPACK_REPOSITORY_MENU_ID(item->id);
+      if (0 <= idx && idx < static_cast<int>(m_repository_addons.size()))
       {
-        log_warning << "Disabling Add-on failed: " << e.what() << std::endl;
+        const Addon& addon = m_addon_manager.get_repository_addon(m_repository_addons[idx]);
+        try
+        {
+          m_addon_manager.install_addon(addon.get_id());
+          m_addon_manager.enable_addon(addon.get_id());
+        }
+        catch(const std::exception& err)
+        {
+          log_warning << "Enabling addon failed: " << err.what() << std::endl;
+        }
+        refresh();
       }
-      set_toggled(item->id, addon.loaded);
     }
   }
+  else
+  {
+       log_warning << "Unknown menu item clicked: " << item->id << std::endl;
+  }
 }
 
 /* EOF */
diff --git a/src/supertux/menu/addon_menu.hpp b/src/supertux/menu/addon_menu.hpp
index 3d81aefdd..715bad64e 100644
--- a/src/supertux/menu/addon_menu.hpp
+++ b/src/supertux/menu/addon_menu.hpp
@@ -32,6 +32,8 @@ private:
 
 private:
   AddonManager& m_addon_manager;
+  std::vector<std::string> m_installed_addons;
+  std::vector<std::string> m_repository_addons;
 
 public:
   AddonMenu();
@@ -40,6 +42,9 @@ public:
   void menu_action(MenuItem* item) override;
 
 private:
+  void rebuild_menu();
+
+private:
   AddonMenu(const AddonMenu&);
   AddonMenu& operator=(const AddonMenu&);
 };
diff --git a/src/supertux/menu/contrib_menu.cpp b/src/supertux/menu/contrib_menu.cpp
index 802c0eac2..8bbc71737 100644
--- a/src/supertux/menu/contrib_menu.cpp
+++ b/src/supertux/menu/contrib_menu.cpp
@@ -33,15 +33,20 @@
 ContribMenu::ContribMenu() :
   m_contrib_worlds()
 {
-  /** Generating contrib levels list by making use of Level Subset  */
+  // Generating contrib levels list by making use of Level Subset
   std::vector<std::string> level_worlds;
-  char** files = PHYSFS_enumerateFiles("levels/");
-  for(const char* const* filename = files; *filename != 0; ++filename) {
-    std::string filepath = std::string("levels/") + *filename;
+
+  std::unique_ptr<char*, decltype(&PHYSFS_freeList)>
+    files(PHYSFS_enumerateFiles("levels"),
+          PHYSFS_freeList);
+  for(const char* const* filename = files.get(); *filename != 0; ++filename)
+  {
+    std::string filepath = FileSystem::join("levels", *filename);
     if(PHYSFS_isDirectory(filepath.c_str()))
+    {
       level_worlds.push_back(filepath);
+    }
   }
-  PHYSFS_freeList(files);
 
   add_label(_("Contrib Levels"));
   add_hl();
diff --git a/src/supertux/world.cpp b/src/supertux/world.cpp
index b6734cc76..cb87b32f3 100644
--- a/src/supertux/world.cpp
+++ b/src/supertux/world.cpp
@@ -43,6 +43,9 @@ World::load(const std::string& directory)
   { // generate savegame filename
     std::string worlddirname = FileSystem::basename(directory);
     std::ostringstream stream;
+#ifdef GRUMBEL
+    // sanitize this!
+#endif
     stream << "profile" << g_config->profile << "/" << worlddirname << ".stsg";
     std::string slotfile = stream.str();
     world->m_savegame_filename = stream.str();