[Widelands-cvs] SF.net SVN: widelands: [3186] trunk/src

[<si...@us...>]

Revision: 3186
          http://widelands.svn.sourceforge.net/widelands/?rev=3186&view=rev
Author:   sigra
Date:     2008-04-21 05:28:05 -0700 (Mon, 21 Apr 2008)

Log Message:
-----------
* In Critter_Bob::roam_update: Only change the values of idle_time_min and idle_time_rnd if they will be used.
* In Graphic::flush: Improve an assertion, get rid of an annoying debug message (seen when creating a new map in the editor) and use iterators in loops.
* In SConscript: Remove reference to a file that no longer exists.
* In RealFSImpl::Write: Initialize variables (and make them const). Fix type of c (was int32_t but fwrite actually returns size_t according to the manual).
* In FileSystem::FS_Filename: Do not subtract 1 before looking at the buffer. Now the function should behave as the comment says.
* In filesystem/zip_filesystem.cc: Attempt to handle errors by throwing an exception with a message from strerror.
* In Basic_Button::handle_mouserelease: Rearrange the function to not touch any member variables after calling send_signal_clicked. This should prevent the kind of bug that was introduced in game_main_menu_save_game.cc in revision 2761 and reverted in revision 3185 (at least from happening directly in Basic_Button::handle_mouserelease).
* In UI::Panel: use regular for loops instead of the somewhat contrived logic involving a prepended if statement and an explicit break statement.
* In UI::Scrollbar::get_knob_pos: Do not return a signed type, which made no sense. Simplify the calculation.

Revision Links:
--------------
    http://widelands.svn.sourceforge.net/widelands/?rev=2761&view=rev
    http://widelands.svn.sourceforge.net/widelands/?rev=3185&view=rev

Modified Paths:
--------------
    trunk/src/SConscript
    trunk/src/critter_bob.cc
    trunk/src/filesystem/disk_filesystem.cc
    trunk/src/filesystem/filesystem.cc
    trunk/src/filesystem/filesystem.h
    trunk/src/filesystem/zip_filesystem.cc
    trunk/src/graphic.cc
    trunk/src/ui/ui_basic/ui_button.cc
    trunk/src/ui/ui_basic/ui_panel.cc
    trunk/src/ui/ui_basic/ui_scrollbar.cc
    trunk/src/ui/ui_basic/ui_scrollbar.h

Modified: trunk/src/SConscript
===================================================================
--- trunk/src/SConscript	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/SConscript	2008-04-21 12:28:05 UTC (rev 3186)
@@ -32,7 +32,6 @@
 libs.reverse()
 
 srcenv.Append(INDENTLIST=[
-	'geometry.h',
 	'helper.h',
 	'helper.cc',
 	'journal.h',

Modified: trunk/src/critter_bob.cc
===================================================================
--- trunk/src/critter_bob.cc	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/critter_bob.cc	2008-04-21 12:28:05 UTC (rev 3186)
@@ -348,9 +348,7 @@
 	Time idle_time_min = 1000;
 	Time idle_time_rnd = CRITTER_MAX_WAIT_TIME_BETWEEN_WALK;
 	if (state->ivar1) {
-		idle_time_min =    1;
-		idle_time_rnd = 1000;
-		state->ivar1  =    0;
+		state->ivar1 = 0;
 		if
 			(start_task_movepath
 			 	(g,
@@ -358,6 +356,7 @@
 			 	 3,
 			 	 descr().get_walk_anims()))
 			return;
+		idle_time_min = 1, idle_time_rnd = 1000;
 	}
 	state->ivar1 = 1;
 	return

Modified: trunk/src/filesystem/disk_filesystem.cc
===================================================================
--- trunk/src/filesystem/disk_filesystem.cc	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/filesystem/disk_filesystem.cc	2008-04-21 12:28:05 UTC (rev 3186)
@@ -384,16 +384,14 @@
 	(std::string const & fname, void const * const data, int32_t const length)
 {
 	std::string fullname;
-	FILE *f;
-	int32_t c;
 
 	fullname=FS_CanonicalizeName(fname);
 
-	f = fopen(fullname.c_str(), "wb");
+	FILE * const f = fopen(fullname.c_str(), "wb");
 	if (!f)
 		throw wexception("Couldn't open %s (%s) for writing", fname.c_str(), fullname.c_str());
 
-	c = fwrite(data, length, 1, f);
+	size_t const c = fwrite(data, length, 1, f);
 	fclose(f);
 
 	if (c != 1)

Modified: trunk/src/filesystem/filesystem.cc
===================================================================
--- trunk/src/filesystem/filesystem.cc	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/filesystem/filesystem.cc	2008-04-21 12:28:05 UTC (rev 3186)
@@ -376,12 +376,9 @@
  * Returns the filename of this path, everything after the last
  * / or \  (or the whole string)
  */
-const char *FileSystem::FS_Filename(const char* buf) {
-	int32_t i=strlen(buf)-1;
-	while (i>=0) {
+char const * FileSystem::FS_Filename(char const * buf) {
+	for (size_t i = strlen(buf); i; --i)
 		if (buf[i]=='/' || buf[i]=='\\') return &buf[i+1];
-		--i;
-	}
 	return buf;
 }
 

Modified: trunk/src/filesystem/filesystem.h
===================================================================
--- trunk/src/filesystem/filesystem.h	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/filesystem/filesystem.h	2008-04-21 12:28:05 UTC (rev 3186)
@@ -111,7 +111,7 @@
 	bool pathIsAbsolute(std::string const & path) const;
 	std::string              AbsolutePath(std::string const & path) const;
 	std::vector<std::string> FS_Tokenize (std::string const & path) const;
-	static const char *FS_Filename(const char* buf);
+	static char const * FS_Filename(char const *);
 	static std::string GetHomedir();
 
 protected:

Modified: trunk/src/filesystem/zip_filesystem.cc
===================================================================
--- trunk/src/filesystem/zip_filesystem.cc	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/filesystem/zip_filesystem.cc	2008-04-21 12:28:05 UTC (rev 3186)
@@ -28,6 +28,7 @@
 #include <cstring>
 #include <cstdlib>
 
+#include <errno.h>
 
 /**
  * Initialize the real file-system
@@ -267,23 +268,30 @@
 	if (complete_file[complete_file.size() - 1] != '/')
 		complete_file += '/';
 
-#ifndef NDEBUG
-	int32_t err =
-#endif
-		zipOpenNewFileInZip3
-		(m_zipfile,
-		 complete_file.c_str(),
-		 &zi,
-		 NULL, 0, NULL, 0, NULL /* comment*/,
-		 Z_DEFLATED,
-		 Z_BEST_COMPRESSION,
-		 0,
-		 -MAX_WBITS,
-		 DEF_MEM_LEVEL,
-		 Z_DEFAULT_STRATEGY,
-		 0,
-		 0);
-	assert(err == ZIP_OK);
+	switch
+		(zipOpenNewFileInZip3
+		 	(m_zipfile,
+		 	 complete_file.c_str(),
+		 	 &zi,
+		 	 NULL, 0, NULL, 0, NULL /* comment*/,
+		 	 Z_DEFLATED,
+		 	 Z_BEST_COMPRESSION,
+		 	 0,
+		 	 -MAX_WBITS,
+		 	 DEF_MEM_LEVEL,
+		 	 Z_DEFAULT_STRATEGY,
+		 	 0,
+		 	 0))
+	{
+	case ZIP_OK:
+		break;
+	case ZIP_ERRNO:
+		throw File_error
+			("ZipFilesystem::MakeDirectory", complete_file, strerror(errno));
+	default:
+		throw File_error
+			("ZipFilesystem::MakeDirectory", complete_file);
+	}
 
 	zipCloseFileInZip(m_zipfile);
 }
@@ -350,18 +358,32 @@
 
 	//  create file
 	std::string const complete_file = m_basename + '/' + fname;
-	int32_t err =
-		zipOpenNewFileInZip3
-		(m_zipfile, complete_file.c_str(), &zi,
-		 NULL, 0, NULL, 0, NULL /* comment*/,
-		 Z_DEFLATED,
-		 Z_BEST_COMPRESSION, 0,
-		 -MAX_WBITS, DEF_MEM_LEVEL, Z_DEFAULT_STRATEGY,
-		 0, 0);
-	assert(err == ZIP_OK);
+	switch
+		(zipOpenNewFileInZip3
+		 	(m_zipfile, complete_file.c_str(), &zi,
+		 	 NULL, 0, NULL, 0, NULL /* comment*/,
+		 	 Z_DEFLATED,
+		 	 Z_BEST_COMPRESSION, 0,
+		 	 -MAX_WBITS, DEF_MEM_LEVEL, Z_DEFAULT_STRATEGY,
+		 	 0, 0))
+	{
+	case ZIP_OK:
+		break;
+	default:
+		throw ZipOperation_error
+			("ZipFilesystem::Write", complete_file, m_zipfilename);
+	}
 
-	err = zipWriteInFileInZip (m_zipfile, data, length);
-	assert(err == ZIP_OK);
+	switch (zipWriteInFileInZip (m_zipfile, data, length)) {
+	case ZIP_OK:
+		break;
+	case ZIP_ERRNO:
+		throw File_error
+			("ZipFilesystem::Write", complete_file, strerror(errno));
+	default:
+		throw File_error
+			("ZipFilesystem::Write", complete_file);
+	}
 
 	zipCloseFileInZip(m_zipfile);
 }

Modified: trunk/src/graphic.cc
===================================================================
--- trunk/src/graphic.cc	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/graphic.cc	2008-04-21 12:28:05 UTC (rev 3186)
@@ -210,50 +210,57 @@
  * \note flush(0) does nothing
 */
 void Graphic::flush(uint8_t const module) {
-	assert(not (module & (PicMod_Font | PicSurface)));
+	assert(not (module & ~(PicMod_UI | PicMod_Menu | PicMod_Game)));
 	// Flush pictures
 
-	for (uint32_t i = 0; i < m_pictures.size(); ++i) {
-		Picture & pic = m_pictures[i];
+	{
+		std::vector<Picture>::const_iterator const pictures_end =
+			m_pictures.end();
+		for
+			(std::vector<Picture>::iterator picture = m_pictures.begin();
+			 picture != pictures_end;
+			 ++picture)
+		{
+			if (!picture->module) {
+				assert(0 == picture->surface);
+				assert(0 == picture->u.fname);
+				continue;
+			}
 
-		//      NoLog("Flushing picture: %i while flushing all!\n", i);
-
-		if (!pic.module)
-			continue;
-
-
-		if (pic.module == PicSurface) {
-			if (!module)
-				log("LEAK: SW16: flush(0): non-picture %i left.\n", i+1);
-
-			continue;
-		}
-
-		pic.module &= ~module; // unmask the modules that should be flushed
-
-		// Once the picture is no longer in any mods, free it
-
-		if (!pic.module) {
-
-			if (pic.u.fname) {
-				m_picturemap.erase(pic.u.fname);
-				free(pic.u.fname);
-				pic.u.fname = 0;
+			// unmask the modules that should be flushed
+			if (! (picture->module &= ~module)) {
+			// Once the picture is no longer in any mods, free it.
+				delete picture->surface; picture->surface = 0;
+				if (picture->u.fname) {
+					m_picturemap.erase(picture->u.fname);
+					free(picture->u.fname); picture->u.fname = 0;
+				}
 			}
-
-			delete pic.surface;
-			pic.surface = 0;
 		}
 	}
 
 	// Flush game items
 	if (!module || module & PicMod_Game) {
-		for (uint32_t i = 0; i < m_maptextures.size(); ++i)
-			delete m_maptextures[i];
+		{
+			std::vector<Texture *>::const_iterator const maptextures_end =
+				m_maptextures.end();
+			for
+				(std::vector<Texture *>::iterator it = m_maptextures.begin();
+				 it != maptextures_end;
+				 ++it)
+			delete *it;
+		}
 		m_maptextures.clear();
 
-		for (uint32_t i = 0; i < m_animations.size(); ++i)
-			delete m_animations[i];
+		{
+			std::vector<AnimationGfx *>::const_iterator const animations_end =
+				m_animations.end();
+			for
+				(std::vector<AnimationGfx *>::iterator it = m_animations.begin();
+				 it != animations_end;
+				 ++it)
+			delete *it;
+		}
 		m_animations.clear();
 
 		delete m_roadtextures;
@@ -446,7 +453,7 @@
 	// Save a png
 	png_structp png_ptr =
 		png_create_write_struct
-		(PNG_LIBPNG_VER_STRING, static_cast<png_voidp>(0), 0, 0);
+			(PNG_LIBPNG_VER_STRING, static_cast<png_voidp>(0), 0, 0);
 
 	if (!png_ptr)
 		throw wexception("Graphic::save_png: Couldn't create png struct!");

Modified: trunk/src/ui/ui_basic/ui_button.cc
===================================================================
--- trunk/src/ui/ui_basic/ui_button.cc	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/ui/ui_basic/ui_button.cc	2008-04-21 12:28:05 UTC (rev 3186)
@@ -241,15 +241,17 @@
 	if (btn != SDL_BUTTON_LEFT) return false;
 
 	if (m_pressed) {
+		m_pressed = false;
 		grab_mouse(false);
+		update(0, 0, get_w(), get_h());
 		if (m_highlighted && m_enabled) {
 			play_click();
 			send_signal_clicked();
+			//  The button may not exist at this point (for example if the button
+			//  closed the dialog that it is part of). So member variables may no
+			//  longer be accessed.
 		}
-		m_pressed = false;
 	}
-	update(0, 0, get_w(), get_h());
-
 	return true;
 }
 };

Modified: trunk/src/ui/ui_basic/ui_panel.cc
===================================================================
--- trunk/src/ui/ui_basic/ui_panel.cc	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/ui/ui_basic/ui_panel.cc	2008-04-21 12:28:05 UTC (rev 3186)
@@ -759,24 +759,24 @@
 	y -= _tborder;
 	if (_flags & pf_top_on_click) move_to_top();
 	if (_g_mousegrab != this)
-		if (Panel * child = _fchild) for (;; child = child->_next) {
-			child = child_at_mouse_cursor(x, y, child);
-			if (not child) break;
+		for
+			(Panel * child = _fchild;
+			 (child = child_at_mouse_cursor(x, y, child));
+			 child = child->_next)
 			if (child->do_mousepress(btn, x - child->_x, y - child->_y))
 				return true;
-		}
 	return handle_mousepress(btn, x, y);
 }
 bool Panel::do_mouserelease(const Uint8 btn, int32_t x, int32_t y) {
 	x -= _lborder;
 	y -= _tborder;
 	if (_g_mousegrab != this)
-		if (Panel * child = _fchild) for (;; child = child->_next) {
-			child = child_at_mouse_cursor(x, y, child);
-			if (not child) break;
+		for
+			(Panel * child = _fchild;
+			 (child = child_at_mouse_cursor(x, y, child));
+			 child = child->_next)
 			if (child->do_mouserelease(btn, x - child->_x, y - child->_y))
 				return true;
-		}
 	return handle_mouserelease(btn, x, y);
 }
 bool Panel::do_mousemove(const Uint8 state, int32_t x, int32_t y, int32_t xdiff, int32_t ydiff)
@@ -784,14 +784,14 @@
 	x -= _lborder;
 	y -= _tborder;
 	if (_g_mousegrab != this)
-		if (Panel * child = _fchild) for (;; child = child->_next) {
-			child = child_at_mouse_cursor(x, y, child);
-			if (not child) break;
+		for
+			(Panel * child = _fchild;
+			 (child = child_at_mouse_cursor(x, y, child));
+			 child = child->_next)
 			if
 				(child->do_mousemove
 				 	(state, x - child->_x, y - child->_y, xdiff, ydiff))
 				return true;
-		}
 	return handle_mousemove(state, x, y, xdiff, ydiff);
 }
 

Modified: trunk/src/ui/ui_basic/ui_scrollbar.cc
===================================================================
--- trunk/src/ui/ui_basic/ui_scrollbar.cc	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/ui/ui_basic/ui_scrollbar.cc	2008-04-21 12:28:05 UTC (rev 3186)
@@ -183,21 +183,12 @@
 /**
  * Return the center of the knob, in pixels, depending on the current position.
 */
-int32_t Scrollbar::get_knob_pos()
-{
-	int32_t extent;
-	uint32_t knobsize = get_knob_size();
-
-	if (m_horizontal)
-		extent = get_w();
-	else
-		extent = get_h();
-
-	extent -= 2 * Size + knobsize; // plus button, minus button, knob size
-
-	if (m_steps == 1)
-		return Size + knobsize/2;
-	return (Size + knobsize/2) + (m_pos * extent) / (m_steps-1);
+uint32_t Scrollbar::get_knob_pos() {
+	assert(0 != m_steps);
+	uint32_t result = Size + get_knob_size() / 2;
+	if (uint32_t const d = m_steps - 1)
+		result += m_pos * ((m_horizontal ? get_w() : get_h()) - 2 * result) / d;
+	return result;
 }
 
 

Modified: trunk/src/ui/ui_basic/ui_scrollbar.h
===================================================================
--- trunk/src/ui/ui_basic/ui_scrollbar.h	2008-04-20 23:03:33 UTC (rev 3185)
+++ trunk/src/ui/ui_basic/ui_scrollbar.h	2008-04-21 12:28:05 UTC (rev 3186)
@@ -64,7 +64,7 @@
 
 private:
 	Area get_area_for_point(int32_t x, int32_t y);
-	int32_t get_knob_pos();
+	uint32_t get_knob_pos();
 	uint32_t get_knob_size();
 	void set_knob_pos(int32_t p);
 


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.