سؤال

لقد كتبت استنساخ Tetris يعمل ولكن لديه تصميم فوضوي للغاية. هل يمكنني الحصول على تعليقات حول كيفية إعادة هيكلة فصولي لجعل ترميزتي أفضل. أركز على جعل الكود الخاص بي عامًا قدر الإمكان ، في محاولة لجعله أكثر محركًا للألعاب باستخدام الكتل فقط.

يتم إنشاء كل كتلة بشكل منفصل في اللعبة. تحتوي لعبتي على قوائم سليمة (قوائم مرتبطة): StaticBlocks و Tetroid. من الواضح أن staticblocks هي قائمة جميع الكتل غير المحزنة ، والكتل الوعائية هي 4 كتل من رباعيات حالية.

  1. في العالم الرئيسي خلق.

  2. أولاً ، يتم إنشاء رباعيات جديدة (4 كتل في قائمة رباعيات) بواسطة (NewTetRoid)

  3. يتم اكتشاف التصادم بواسطة وظائف (*** Collide) ، من خلال مقارنة كل من رباعيات مع جميع الحواجز الثابتة باستخدام وظائف (if *****).

  4. عندما يتوقف رباعيات الروماني (يضرب القاع/الكتل) ، يتم نسخه (نسخ نسخ) إلى الحواجز الثابتة ويتم تصنيع رباعي الفرق ، ثم يتم إجراء الاختبارات للخطوط الكاملة ، يتم تدمير/إسقاط الكتل وما إلى ذلك عن طريق البحث في الحواجز الثابتة مع (البحث).

  5. يتم إنشاء رباعي جديد.

(Translatetetroid) و (rotatetetroide) تنفيذ العمليات على كل كتلة في قائمة رباعيات واحد تلو الآخر ( أعتقد أن هذه ممارسة سيئة).

(DrawBlockList) يمر فقط من خلال قائمة ، تشغيل وظيفة Draw () لكل كتلة.

يتم التحكم في الدوران عن طريق ضبط محور الدوران بالنسبة للكتلة الأولى في رباعيات عندما يتم استدعاء (newTetroid). تدور وظيفة الدوران الخاصة بي (تدوير) لكل كتلة حول المحور ، باستخدام إدخال +-1 للتناوب الأيسر/الأيمن. تعتبر توريدات الدوران وحالات الكتل التي تدور بطريقتين أو 4 طرق مختلفة ، وتحدد الحالة التي تقع فيها حاليًا ، وما إذا كان ينبغي تدويرها يسارًا أو يمينًا. أنا لست سعيدًا بكيفية تعريفها في "World" ، لكنني لا أعرف مكان وضعها ، مع الحفاظ على وظيفة (تدوير) عام لكل كتلة.

فصولي كما يلي

class World
{
    public:
    /* Constructor/Destructor */
    World();
    ~World();

    /* Blocks Operations */
    void AppendBlock(int, int, BlockList&);
    void RemoveBlock(Block*, BlockList&);;

    /* Tetroid Operations */
    void NewTetroid(int, int, int, BlockList&);
    void TranslateTetroid(int, int, BlockList&);
    void RotateTetroid(int, BlockList&);
    void CopyTetroid(BlockList&, BlockList&);

    /* Draw */
    void DrawBlockList(BlockList&);
    void DrawWalls();

    /* Collisions */
    bool TranslateCollide(int, int, BlockList&, BlockList&);
    bool RotateCollide(int, BlockList&, BlockList&);
    bool OverlapCollide(BlockList&, BlockList&); // For end of game

    /* Game Mechanics */
    bool CompleteLine(BlockList&); // Test all line
    bool CompleteLine(int, BlockList&); // Test specific line
    void ColourLine(int, BlockList&);
    void DestroyLine(int, BlockList&);
    void DropLine(int, BlockList&); // Drops all blocks above line

    int rotationAxisX;
    int rotationAxisY;
    int rotationState; // Which rotation it is currently in
    int rotationModes; // How many diff rotations possible

    private:
    int wallX1;
    int wallX2;
    int wallY1;
    int wallY2;
};

class BlockList
{
    public:
    BlockList();
    ~BlockList();

    Block* GetFirst();
    Block* GetLast();

    /* List Operations */
    void Append(int, int);
    int  Remove(Block*);
    int  SearchY(int);

    private:
    Block *first;
    Block *last;
};

class Block
{
    public:
    Block(int, int);
    ~Block();

    int GetX();
    int GetY();

    void SetColour(int, int, int);

    void Translate(int, int);
    void Rotate(int, int, int);

    /* Return values simulating the operation (for collision purposes) */
    int IfTranslateX(int);
    int IfTranslateY(int);
    int IfRotateX(int, int, int);
    int IfRotateY(int, int, int);

    void Draw();

    Block *next;

    private:
    int pX; // position x
    int pY; // position y
    int colourR;
    int colourG;
    int colourB;
};

آسف إذا كان هذا غير واضح بعض الشيء أو طويلًا ، فأنا أبحث عن بعض المساعدة في إعادة الهيكلة.

هل كانت مفيدة؟

المحلول

  • ما هو غير متزوج مسؤولية World صف دراسي؟ إنها مجرد نقطة تحتوي على كل نوع من الوظائف. هذا ليس تصميمًا جيدًا. واحدة من المسؤولية الواضحة هي "تمثيل الشبكة التي توضع عليها الكتل". ولكن هذا لا علاقة له بإنشاء رباعيات أو قوائم كتلة معالجة أو رسم. في الواقع ، ربما لا يلزم أن يكون في فصل على الإطلاق. أتوقع World كائن لاحتواء BlockList يمكنك الاتصال بـ StaticBlocks حتى تتمكن من تحديد الشبكة التي تلعب عليها.
  • لماذا تحدد خاصتك Blocklist؟ قلت إنك تريد أن يكون الرمز الخاص بك عامًا ، فلماذا لا تسمح أي حاوية لاستخدامها؟ لماذا لا يمكنني استخدام أ std::vector<Block> إذا أردت ذلك؟ أو أ std::set<Block>, أو بعض الحاوية المحنطة بالمنزل؟
  • استخدم أسماء بسيطة لا تكرر المعلومات أو تتناقض مع نفسها. TranslateTetroid لا يترجم رباعي. يترجم جميع الكتل في قائمة الكتلة. لذلك ينبغي أن يكون TranslateBlocks أو شيء ما. ولكن حتى هذا زائد. يمكننا أن نرى من التوقيع (يستغرق BlockList&) أنه يعمل على الكتل. لذا فقط اتصل به Translate.
  • حاول تجنب التعليقات على غرار C (/*...*/). C ++-النمط (//..) يتصرف بشكل أجمل قليلاً في أنه إذا كنت تستخدم تعليقًا على غرار C ، فسيتم كسره إذا كانت هذه الكتلة تحتوي أيضًا على تعليقات على غرار C. (كمثال بسيط ، /*/**/*/ لن يعمل ، لأن المترجم سيرى الأول */ كنهاية التعليق ، وهكذا الأخير */ لن يعتبر تعليقًا.
  • ما هو مع كل (لم يكشف عن اسمه) int المعلمات؟ إنه يجعل قراءة الكود الخاص بك مستحيلًا.
  • احترام ميزات اللغة والاتفاقيات. طريقة نسخ كائن تستخدم مُنشئ النسخ الخاص به. لذلك بدلا من أ CopyTetroid وظيفة ، إعطاء BlockList منشئ نسخة. ثم إذا كنت بحاجة إلى نسخ واحدة ، يمكنني ببساطة القيام بذلك BlockList b1 = b0.
  • عوضا عن void SetX(Y) و Y GetX() الأساليب ، إسقاط بادئة/مجموعة GET الزائدة عن الحاجة وببساطة لديك void X(Y) و Y X(). نحن نعلم أنها getter لأنها لا تتطلب أي معلمات وإرجاع قيمة. ونحن نعلم أن الآخر هو وسيلة لأنها تأخذ معلمة وإرجاع باطلة.
  • BlockList ليس تجريدًا جيدًا جدًا. لديك احتياجات مختلفة جدًا لـ "tetroid الحالي" و "قائمة الكتل الثابتة حاليًا على الشبكة". يمكن تمثيل الكتل الثابتة بتسلسل بسيط من الكتل كما لديك (على الرغم من أن تسلسل الصفوف ، أو صفيف ثنائي الأبعاد ، قد يكون أكثر ملاءمة) ، ولكن يحتاج رباعي رباعي نشط حاليًا إلى معلومات إضافية ، مثل مركز الدوران (الذي لا ينتمي في World).
    • قد تكون هناك طريقة بسيطة لتمثيل رباعي الأسنان ، وتخفيف الدورات ، لتخزين كتل الأعضاء إزاحة بسيطة من مركز الدوران. وهذا يجعل التناوب أسهل في حساب ، ويعني أنه لا يجب تحديث كتل الأعضاء على الإطلاق أثناء الترجمة. فقط مركز الدوران يجب نقله.
    • في القائمة الثابتة ، ليس من الفعال للكتل لمعرفة موقعها. بدلاً من ذلك ، يجب أن تقوم الشبكة بتخطيط المواقع إلى كتل (إذا سألت الشبكة "التي توجد كتلة في الخلية (5,8), ، يجب أن تكون قادرة على إعادة الكتلة. لكن الكتلة نفسها لا تحتاج إلى تخزين الإحداثيات. إذا كان الأمر كذلك ، فقد يصبح صداع الصيانة. ماذا لو ، بسبب بعض الأخطاء الدقيقة ، تنتهي كتلتين بنفس الإحداثيات؟ يمكن أن يحدث ذلك إذا قامت Blocks بتخزين إحداثياتها الخاصة ، ولكن ليس إذا كانت الشبكة تحتوي على قائمة بلوك هي المكان.)
    • هذا يخبرنا أننا نحتاج إلى تمثيل واحد لـ "كتلة ثابتة" ، وآخر لـ "كتلة ديناميكية" (يحتاج إلى تخزين الإزاحة من مركز رباعي الأسنان). في الواقع ، يمكن غلي الكتلة "الثابتة" إلى الأساسيات: إما خلية في الشبكة تحتوي على كتلة ، وأن هذه الكتلة لها لون ، أو لا تحتوي على كتلة. لا يوجد سلوك آخر مرتبط بهذه الكتل ، لذلك ربما تكون الخلية التي يتم وضعها فيها والتي يجب تصميمها بدلاً من ذلك.
    • ونحن بحاجة إلى فئة تمثل رباعيًا متحركًا/ديناميكيًا.
  • نظرًا لأن الكثير من اكتشاف الاصطدام الخاص بك "تنبؤية" لأنه يتعامل مع "ماذا لو قمت بنقل الكائن هنا" ، فقد يكون من الأسهل تنفيذ وظائف الترجمة/التناوب غير المتقدمة. يجب أن تترك هذه الكائن الأصلي غير المعدل ، وإرجاع نسخة متداولة/مترجمة.

لذا ، إليك تمريرة أولى على الكود الخاص بك ، ما عليك سوى إعادة تسمية التعليمات البرمجية والتعليق عليها وإزالة الكود دون تغيير الهيكل أكثر من اللازم.

class World
{
public:
    // Constructor/Destructor
    // the constructor should bring the object into a useful state. 
    // For that, it needs to know the dimensions of the grid it is creating, does it not?
    World(int width, int height);
    ~World();

    // none of thes have anything to do with the world
    ///* Blocks Operations */
    //void AppendBlock(int, int, BlockList&);
    //void RemoveBlock(Block*, BlockList&);;

    // Tetroid Operations
    // What's wrong with using BlockList's constructor for, well, constructing BlockLists? Why do you need NewTetroid?
    //void NewTetroid(int, int, int, BlockList&);

    // none of these belong in the World class. They deal with BlockLists, not the entire world.
    //void TranslateTetroid(int, int, BlockList&);
    //void RotateTetroid(int, BlockList&);
    //void CopyTetroid(BlockList&, BlockList&);

    // Drawing isn't the responsibility of the world
    ///* Draw */
    //void DrawBlockList(BlockList&);
    //void DrawWalls();

    // these are generic functions used to test for collisions between any two blocklists. So don't place them in the grid/world class.
    ///* Collisions */
    //bool TranslateCollide(int, int, BlockList&, BlockList&);
    //bool RotateCollide(int, BlockList&, BlockList&);
    //bool OverlapCollide(BlockList&, BlockList&); // For end of game

    // given that these functions take the blocklist on which they're operating as an argument, why do they need to be members of this, or any, class?
    // Game Mechanics 
    bool AnyCompleteLines(BlockList&); // Renamed. I assume that it returns true if *any* line is complete?
    bool IsLineComplete(int line, BlockList&); // Renamed. Avoid ambiguous names like "CompleteLine". is that a command? (complete this line) or a question (is this line complete)?
    void ColourLine(int line, BlockList&); // how is the line supposed to be coloured? Which colour?
    void DestroyLine(int line, BlockList&); 
    void DropLine(int, BlockList&); // Drops all blocks above line

    // bad terminology. The objects are rotated about the Z axis. The x/y coordinates around which it is rotated are not axes, just a point.
    int rotationAxisX;
    int rotationAxisY;
    // what's this for? How many rotation states exist? what are they?
    int rotationState; // Which rotation it is currently in
    // same as above. What is this, what is it for?
    int rotationModes; // How many diff rotations possible

private:
    int wallX1;
    int wallX2;
    int wallY1;
    int wallY2;
};

// The language already has perfectly well defined containers. No need to reinvent the wheel
//class BlockList
//{
//public:
//  BlockList();
//  ~BlockList();
//
//  Block* GetFirst();
//  Block* GetLast();
//
//  /* List Operations */
//  void Append(int, int);
//  int  Remove(Block*);
//  int  SearchY(int);
//
//private:
//  Block *first;
//  Block *last;
//};

struct Colour {
    int r, g, b;
};

class Block
{
public:
    Block(int x, int y);
    ~Block();

    int X();
    int Y();

    void Colour(const Colour& col);

    void Translate(int down, int left); // add parameter names so we know the direction in which it is being translated
    // what were the three original parameters for? Surely we just need to know how many 90-degree rotations in a fixed direction (clockwise, for example) are desired?
    void Rotate(int cwSteps); 

    // If rotate/translate is non-mutating and instead create new objects, we don't need these predictive collision functions.x ½
    //// Return values simulating the operation (for collision purposes) 
    //int IfTranslateX(int);
    //int IfTranslateY(int);
    //int IfRotateX(int, int, int);
    //int IfRotateY(int, int, int);

    // the object shouldn't know how to draw itself. That's building an awful lot of complexity into the class
    //void Draw();

    //Block *next; // is there a next? How come? What does it mean? In which context? 

private:
    int x; // position x
    int y; // position y
    Colour col;
    //int colourR;
    //int colourG;
    //int colourB;
};

// Because the argument block is passed by value it is implicitly copied, so we can modify that and return it
Block Translate(Block bl, int down, int left) {
    return bl.Translate(down, left);
}
Block Rotate(Block bl, cwSteps) {
    return bl.Rotate(cwSteps);
}

الآن ، دعنا نضيف بعض القطع المفقودة:

أولاً ، سنحتاج إلى تمثيل الكتل "الديناميكية" ، والكتل التي يمتلكها ، والكتل الثابتة أو الخلايا في الشبكة. (سنضيف أيضًا طريقة "تصادم" بسيطة إلى فئة العالم/الشبكة)

class Grid
{
public:
    // Constructor/Destructor
    Grid(int width, int height);
    ~Grid();

    // perhaps these should be moved out into a separate "game mechanics" object
    bool AnyCompleteLines();
    bool IsLineComplete(int line);
    void ColourLine(int line, Colour col);Which colour?
    void DestroyLine(int line); 
    void DropLine(int);

    int findFirstInColumn(int x, int y); // Starting from cell (x,y), find the first non-empty cell directly below it. This corresponds to the SearchY function in the old BlockList class
    // To find the contents of cell (x,y) we can do cells[x + width*y]. Write a wrapper for this:
    Cell& operator()(int x, int y) { return cells[x + width*y]; }
    bool Collides(Tetroid& tet); // test if a tetroid collides with the blocks currently in the grid

private:
    // we can compute the wall positions on demand from the grid dimensions
    int leftWallX() { return 0; }
    int rightWallX() { return width; }
    int topWallY() { return 0; }
    int bottomWallY { return height; }

    int width;
    int height;

    // let this contain all the cells in the grid. 
    std::vector<Cell> cells; 

};

// represents a cell in the game board grid
class Cell {
public:
    bool hasBlock();
    Colour Colour();
};

struct Colour {
    int r, g, b;
};

class Block
{
public:
    Block(int x, int y, Colour col);
    ~Block();

    int X();
    int Y();
void X(int);
void Y(int);

    void Colour(const Colour& col);

private:
    int x; // x-offset from center
    int y; // y-offset from center
    Colour col; // this could be moved to the Tetroid class, if you assume that tetroids are always single-coloured
};

class Tetroid { // since you want this generalized for more than just Tetris, perhaps this is a bad name
public:
    template <typename BlockIter>
    Tetroid(BlockIter first, BlockIter last); // given a range of blocks, as represented by an iterator pair, store the blocks in the tetroid

    void Translate(int down, int left) { 
        centerX += left; 
        centerY += down;
    }
    void Rotate(int cwSteps) {
        typedef std::vector<Block>::iterator iter;
        for (iter cur = blocks.begin(); cur != blocks.end(); ++cur){
            // rotate the block (*cur) cwSteps times 90 degrees clockwise.
                    // a naive (but inefficient, especially for large rotations) solution could be this:
        // while there is clockwise rotation left to perform
        for (; cwSteps > 0; --cwSteps){
            int x = -cur->Y(); // assuming the Y axis points downwards, the new X offset is simply the old Y offset negated
            int y = cur->X(); // and the new Y offset is the old X offset unmodified
            cur->X(x);
            cur->Y(y);
        }
        // if there is any counter-clockwise rotation to perform (if cwSteps was negative)
        for (; cwSteps < 0; --cwSteps){
            int x = cur->Y();
            int y = -cur->X();
            cur->X(x);
            cur->Y(y);
        }
        }
    }

private:
    int centerX, centerY;
    std::vector<Block> blocks;
};

Tetroid Translate(Tetroid tet, int down, int left) {
    return tet.Translate(down, left);
}
Tetroid Rotate(Tetroid tet, cwSteps) {
    return tet.Rotate(cwSteps);
}

وسنحتاج إلى إعادة تنفيذ فحص التصادم المضاربة. بالنظر إلى طرق الترجمة/التدوير غير المقلدة ، وهذا أمر بسيط: نقوم فقط بإنشاء نسخ متداولة/مترجمة ، واختبار تلك الخاصة بالتصادم:

// test if a tetroid t would collide with the grid g if it was translated (x,y) units
if (g.Collides(Translate(t, x, y))) { ... }

// test if a tetroid t would collide with the grid g if it was rotated x times clockwise
if (g.Collides(Rotate(t, x))) { ... }

نصائح أخرى

أود شخصيا التخلص من الكتل الثابتة والتعامل معها كصفوف. وجود كتلة ثابتة ، تحافظ على معلومات أكثر بكثير مما تحتاج.

عالم مصنوع من صفوف ، وهي مجموعة من المربعات المفردة. يمكن أن تكون المربعات إما فارغة ، أو لون (أو تمديدها إذا كان لديك كتل خاصة).

يمتلك العالم أيضًا كتلة نشطة واحدة ، كما لديك الآن. يجب أن يكون للفصل طريقة تدوير وترجمة. من الواضح أن الكتلة ستحتاج إلى الحفاظ على إشارة إلى العالم لتحديد ما إذا كان سيتم تصادمه بالطوب الحالي أو حافة اللوحة.

عندما تخرج الكتلة النشطة عن اللعب ، ستطلق على شيء مثل العالم. كتلة نشطة جديدة إذا لزم الأمر.

مرخصة بموجب: CC-BY-SA مع الإسناد
لا تنتمي إلى StackOverflow
scroll top