SCI: Minor cleanup

1. Reorder member initialisations to match class member order
2. Use #pragma mark instead of comments for annotating sections
3. Remove useless >=0 checks on unsigned types

engines/sci/graphics/celobj32.cpp

@@ -145,8 +145,8 @@ struct SCALER_Scale {
 	// so just always make the reader decompress an entire
 	// line of source data when scaling
 	_reader(celObj, celObj._width),
-	_lastIndex(maxWidth - 1),
-	_table(CelObj::_scaler->getScalerTable(scaleX, scaleY)) {}
+	_table(CelObj::_scaler->getScalerTable(scaleX, scaleY)),
+	_lastIndex(maxWidth - 1) {}
 
 	inline void setSource(const int16 x, const int16 y) {
 		_row = _reader.getRow(_table->valuesY[y]);
@@ -173,12 +173,10 @@ struct READER_Uncompressed {
 private:
 	byte *_pixels;
 	const int16 _sourceWidth;
-	const int16 _maxWidth;
 
 public:
-	READER_Uncompressed(const CelObj &celObj, const int16 maxWidth) :
-	_sourceWidth(celObj._width),
-	_maxWidth(maxWidth) {
+	READER_Uncompressed(const CelObj &celObj, const int16) :
+	_sourceWidth(celObj._width) {
 		byte *resource = celObj.getResPointer();
 		_pixels = resource + READ_SCI11ENDIAN_UINT32(resource + celObj._celHeaderOffset + 24);
 	}
@@ -202,11 +200,11 @@ private:
 
 public:
 	READER_Compressed(const CelObj &celObj, const int16 maxWidth) :
-	_y(-1),
-	_maxWidth(maxWidth),
 	_resource(celObj.getResPointer()),
+	_y(-1),
 	_sourceHeight(celObj._height),
-	_transparentColor(celObj._transparentColor) {
+	_transparentColor(celObj._transparentColor),
+	_maxWidth(maxWidth) {
 		assert(_maxWidth <= celObj._width);
 
 		byte *celHeader = _resource + celObj._celHeaderOffset;

engines/sci/graphics/frameout.cpp

@@ -70,22 +70,22 @@ static int16 unknownCDefaults[2][16] = {
 };
 
 GfxFrameout::GfxFrameout(SegManager *segMan, ResourceManager *resMan, GfxCoordAdjuster *coordAdjuster, GfxCache *cache, GfxScreen *screen, GfxPalette32 *palette, GfxPaint32 *paint32) :
-	_segMan(segMan),
-	_resMan(resMan),
-	_cache(cache),
-	_screen(screen),
-	_palette(palette),
-	_paint32(paint32),
 	_isHiRes(false),
-	_palMorphIsOn(false),
+	_cache(cache),
+	_palette(palette),
+	_resMan(resMan),
+	_screen(screen),
+	_segMan(segMan),
+	_paint32(paint32),
 	_showStyles(nullptr),
-	_remapOccurred(false),
-	_frameNowVisible(false),
 	// TODO: Stop using _gfxScreen
 	_currentBuffer(screen->getWidth(), screen->getHeight(), nullptr),
 	_priorityMap(screen->getWidth(), screen->getHeight(), nullptr),
+	_remapOccurred(false),
+	_frameNowVisible(false),
 	_screenRect(screen->getWidth(), screen->getHeight()),
-	_overdrawThreshold(0) {
+	_overdrawThreshold(0),
+	_palMorphIsOn(false) {
 
 	_currentBuffer.setPixels(calloc(1, screen->getWidth() * screen->getHeight()));
 

engines/sci/graphics/lists32.h

@@ -76,12 +76,12 @@ public:
 	}
 
 	T *const &operator[](size_type index) const {
-		assert(index >= 0 && index < _size);
+		assert(index < _size);
 		return _items[index];
 	}
 
 	T *&operator[](size_type index) {
-		assert(index >= 0 && index < _size);
+		assert(index < _size);
 		return _items[index];
 	}
 
@@ -144,7 +144,7 @@ public:
 	 * Erases the object pointed to at the given index.
 	 */
 	void erase_at(size_type index) {
-		assert(index >= 0 && index < _size);
+		assert(index < _size);
 
 		delete _items[index];
 		_items[index] = nullptr;

engines/sci/graphics/palette32.cpp

@@ -34,17 +34,26 @@ namespace Sci {
 
 GfxPalette32::GfxPalette32(ResourceManager *resMan, GfxScreen *screen)
 	: GfxPalette(resMan, screen),
-	_clutTable(nullptr),
-	// Palette cycling
-	_cyclers(), _cycleMap(),
-	// Palette varying
-	_sourcePalette(_sysPalette), _nextPalette(_sysPalette),
-	_varyTime(0), _varyDirection(0), _varyTargetPercent(0),
-	_varyTargetPalette(nullptr), _varyStartPalette(nullptr),
-	_varyFromColor(0), _varyToColor(255), _varyNumTimesPaused(0),
-	_varyLastTick(0),
 	// Palette versioning
-	_version(1), _versionUpdated(false) {
+	_version(1),
+	_versionUpdated(false),
+	_sourcePalette(_sysPalette),
+	_nextPalette(_sysPalette),
+	// Clut
+	_clutTable(nullptr),
+	// Palette varying
+	_varyStartPalette(nullptr),
+	_varyTargetPalette(nullptr),
+	_varyFromColor(0),
+	_varyToColor(255),
+	_varyLastTick(0),
+	_varyTime(0),
+	_varyDirection(0),
+	_varyTargetPercent(0),
+	_varyNumTimesPaused(0),
+	// Palette cycling
+	_cyclers(),
+	_cycleMap() {
 		_varyPercent = _varyTargetPercent;
 		memset(_fadeTable, 100, sizeof(_fadeTable));
 		// NOTE: In SCI engine, the palette manager constructor loads
@@ -258,9 +267,8 @@ void GfxPalette32::applyAll() {
 	applyFade();
 }
 
-//
-// Clut
-//
+#pragma mark -
+#pragma mark Colour look-up
 
 bool GfxPalette32::loadClut(uint16 clutId) {
 	// loadClut() will load a color lookup table from a clu file and set
@@ -312,9 +320,8 @@ void GfxPalette32::unloadClut() {
 	_clutTable = nullptr;
 }
 
-//
-// Palette vary
-//
+#pragma mark -
+#pragma mark Varying
 
 inline bool GfxPalette32::createPaletteFromResourceInternal(const GuiResourceId paletteId, Palette *const out) const {
 	Resource *palResource = _resMan->findResource(ResourceId(kResourceTypePalette, paletteId), false);
@@ -554,9 +561,8 @@ void GfxPalette32::applyVary() {
 	}
 }
 
-//
-// Palette cycling
-//
+#pragma mark -
+#pragma mark Cycling
 
 inline void GfxPalette32::clearCycleMap(const uint16 fromColor, const uint16 numColorsToClear) {
 	bool *mapEntry = _cycleMap + fromColor;
@@ -780,9 +786,8 @@ void GfxPalette32::applyCycles() {
 	}
 }
 
-//
-// Palette fading
-//
+#pragma mark -
+#pragma mark Fading
 
 // NOTE: There are some game scripts (like SQ6 Sierra logo and main menu) that call
 // setFade with numColorsToFade set to 256, but other parts of the engine like

engines/sci/graphics/plane32.cpp

@@ -44,18 +44,18 @@ void DrawList::add(ScreenItem *screenItem, const Common::Rect &rect) {
 uint16 Plane::_nextObjectId = 20000;
 
 Plane::Plane(const Common::Rect &gameRect) :
-_gameRect(gameRect),
-_object(make_reg(0, _nextObjectId++)),
-_back(0),
-_pictureId(kPlanePicColored),
-_mirrored(false),
 _width(g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth),
 _height(g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight),
-_deleted(0),
-_updated(0),
+_pictureId(kPlanePicColored),
+_mirrored(false),
+_back(0),
 _priorityChanged(0),
+_object(make_reg(0, _nextObjectId++)),
+_redrawAllCount(g_sci->_gfxFrameout->getScreenCount()),
 _created(g_sci->_gfxFrameout->getScreenCount()),
-_redrawAllCount(g_sci->_gfxFrameout->getScreenCount()) {
+_updated(0),
+_deleted(0),
+_gameRect(gameRect) {
 	convertGameRectToPlaneRect();
 	_priority = MAX(10000, g_sci->_gfxFrameout->getPlanes().getTopPlanePriority() + 1);
 	setType();
@@ -63,14 +63,14 @@ _redrawAllCount(g_sci->_gfxFrameout->getScreenCount()) {
 }
 
 Plane::Plane(reg_t object) :
-_object(object),
 _width(g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth),
 _height(g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight),
-_created(g_sci->_gfxFrameout->getScreenCount()),
-_redrawAllCount(g_sci->_gfxFrameout->getScreenCount()),
-_deleted(0),
-_updated(0),
 _priorityChanged(false),
+_object(object),
+_redrawAllCount(g_sci->_gfxFrameout->getScreenCount()),
+_created(g_sci->_gfxFrameout->getScreenCount()),
+_updated(0),
+_deleted(0),
 _moved(0) {
 	SegManager *segMan = g_sci->getEngineState()->_segMan;
 	_vanishingPoint.x = readSelectorValue(segMan, object, SELECTOR(vanishingX));
@@ -93,13 +93,13 @@ _moved(0) {
 }
 
 Plane::Plane(const Plane &other) :
-_object(other._object),
-_priority(other._priority),
 _pictureId(other._pictureId),
 _mirrored(other._mirrored),
-_back(other._back),
 _field_34(other._field_34), _field_38(other._field_38),
 _field_3C(other._field_3C), _field_40(other._field_40),
+_back(other._back),
+_object(other._object),
+_priority(other._priority),
 _planeRect(other._planeRect),
 _gameRect(other._gameRect),
 _screenRect(other._screenRect),

engines/sci/graphics/screen_item32.cpp

@@ -36,13 +36,13 @@ namespace Sci {
 uint16 ScreenItem::_nextObjectId = 20000;
 
 ScreenItem::ScreenItem(const reg_t object) :
-_mirrorX(false),
-_pictureId(-1),
 _celObj(nullptr),
 _object(object),
-_deleted(0),
+_pictureId(-1),
+_created(g_sci->_gfxFrameout->getScreenCount()),
 _updated(0),
-_created(g_sci->_gfxFrameout->getScreenCount()) {
+_deleted(0),
+_mirrorX(false) {
 	SegManager *segMan = g_sci->getEngineState()->_segMan;
 
 	setFromObject(segMan, object, true, true);
@@ -50,65 +50,65 @@ _created(g_sci->_gfxFrameout->getScreenCount()) {
 }
 
 ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo) :
-_position(0, 0),
-_z(0),
-_object(make_reg(0, _nextObjectId++)),
-_celInfo(celInfo),
 _plane(plane),
-_celObj(nullptr),
 _useInsetRect(false),
+_z(0),
+_celInfo(celInfo),
+_celObj(nullptr),
 _fixPriority(false),
-_mirrorX(false),
+_position(0, 0),
+_object(make_reg(0, _nextObjectId++)),
 _pictureId(-1),
+_created(g_sci->_gfxFrameout->getScreenCount()),
 _updated(0),
 _deleted(0),
-_created(g_sci->_gfxFrameout->getScreenCount()) {}
+_mirrorX(false) {}
 
 ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo, const Common::Rect &rect) :
-_position(rect.left, rect.top),
-_z(0),
-_object(make_reg(0, _nextObjectId++)),
-_celInfo(celInfo),
 _plane(plane),
-_celObj(nullptr),
 _useInsetRect(false),
+_z(0),
+_celInfo(celInfo),
+_celObj(nullptr),
 _fixPriority(false),
-_mirrorX(false),
+_position(rect.left, rect.top),
+_object(make_reg(0, _nextObjectId++)),
 _pictureId(-1),
+_created(g_sci->_gfxFrameout->getScreenCount()),
 _updated(0),
 _deleted(0),
-_created(g_sci->_gfxFrameout->getScreenCount()) {
+_mirrorX(false) {
 	if (celInfo.type == kCelTypeColor) {
 		_insetRect = rect;
 	}
 }
 
 ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo, const Common::Rect &rect, const ScaleInfo &scaleInfo) :
-_position(rect.left, rect.top),
-_z(0),
-_object(make_reg(0, _nextObjectId++)),
-_celInfo(celInfo),
 _plane(plane),
-_celObj(nullptr),
+_scale(scaleInfo),
 _useInsetRect(false),
+_z(0),
+_celInfo(celInfo),
+_celObj(nullptr),
 _fixPriority(false),
-_mirrorX(false),
+_position(rect.left, rect.top),
+_object(make_reg(0, _nextObjectId++)),
 _pictureId(-1),
+_created(g_sci->_gfxFrameout->getScreenCount()),
 _updated(0),
 _deleted(0),
-_created(g_sci->_gfxFrameout->getScreenCount()),
-_scale(scaleInfo) {}
+_mirrorX(false) {}
 
 ScreenItem::ScreenItem(const ScreenItem &other) :
-_object(other._object),
 _plane(other._plane),
+_scale(other._scale),
+_useInsetRect(other._useInsetRect),
 _celInfo(other._celInfo),
 _celObj(nullptr),
-_screenRect(other._screenRect),
+_object(other._object),
 _mirrorX(other._mirrorX),
-_useInsetRect(other._useInsetRect),
-_scale(other._scale),
-_scaledPosition(other._scaledPosition) {
+_scaledPosition(other._scaledPosition),
+_screenRect(other._screenRect) {
 	if (other._useInsetRect) {
 		_insetRect = other._insetRect;
 	}