Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actor z-index throws an error if set on a child actor or on an Actor not part of a scene #888

Closed
eonarheim opened this issue Sep 27, 2017 · 1 comment · Fixed by #1679
Closed
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior

Comments

@eonarheim
Copy link
Member

eonarheim commented Sep 27, 2017

This issue was brought up on the forum by Maxime, https://groups.google.com/forum/#!topic/excaliburjs/NPdFFSPDdwE

This may take some work to fix, probably by generalizing draw code from scene and extracting them into a separate class that can be used both by scenes, child actors, and any other actor based data structrue meant for drawing.

Steps to Reproduce

Repro codepen https://codepen.io/anon/pen/YrVwOe?editors=0010#0

image

Setting the z-index on a child actor does not work because the scene drawTree relies on a scene to order drawing, but child actors do not have a scene they are a member of.

Excalibur Actor.ts line 620

   /**
    * Sets the z-index of an actor and updates it in the drawing list for the scene. 
    * The z-index determines the relative order an actor is drawn in.
    * Actors with a higher z-index are drawn on top of actors with a lower z-index
    * @param newIndex new z-index to assign
    */
   public setZIndex(newIndex: number) {
      this.scene.cleanupDrawTree(this);
      this._zIndex = newIndex;
      this.scene.updateDrawTree(this);
   }

Expected Result

Excalibur should allow z-index operations in child actors

Actual Result

Errors are thrown and the game does not load.

Environment

  • browsers and versions: Chrome 60+
  • operating system: Windows 10
  • Excalibur versions: v0.12.0
@eonarheim eonarheim added the bug This issue describes undesirable, incorrect, or unexpected behavior label Sep 27, 2017
@eonarheim eonarheim added this to the 0.13.0 Release milestone Sep 27, 2017
@jedeen jedeen modified the milestones: 0.13.0 Release, 0.14.0 Release Oct 7, 2017
@jedeen jedeen modified the milestones: 0.14.0 Release, 0.15.0 Release Dec 1, 2017
@eonarheim eonarheim changed the title Actor z-index throws an error if set on a child actor Actor z-index throws an error if set on a child actor or on an Actor not part of a scene Feb 12, 2018
@eonarheim
Copy link
Member Author

This also happens if the actor is not part of scene when you set the z-index

@jedeen jedeen modified the milestones: 0.15.0 Release, 0.16.0 Release Feb 13, 2018
@jedeen jedeen self-assigned this Feb 13, 2018
@jedeen jedeen modified the milestones: 0.16.0 Release, 0.17.0 Release Mar 31, 2018
@jedeen jedeen modified the milestones: 0.17.0 Release, 0.18.0 Release Jun 6, 2018
@jedeen jedeen modified the milestones: 0.18.0 Release, 0.19.0 Release Aug 3, 2018
@jedeen jedeen modified the milestones: 0.19.0 Release, 0.20.0 Release Oct 13, 2018
@jedeen jedeen modified the milestones: 0.20.0 Release, 0.21.0 Release Dec 11, 2018
@jedeen jedeen modified the milestones: 0.22.0 Release, 0.23.0 Release Apr 6, 2019
@jedeen jedeen removed their assignment Jun 29, 2019
@jedeen jedeen modified the milestones: 0.24.0 Release, 0.25.0 Nov 24, 2019
eonarheim added a commit that referenced this issue Oct 4, 2020
Closes #1678 
Also Closes #888 

## Changes:

- Fix z-index regression in sorted list
- Add safety around z-index setting that fixes #888
- Adds a unit test to ensure this isn't broken in the future
eonarheim added a commit that referenced this issue Nov 5, 2020
This change comes with a lot of advantages, everything including TileMaps can make use of z-indexing, anything with a pos, rotation, scale, and draw() method can be drawn.

Related #1361
Closes #1018 #888 

Docs PR: excaliburjs/excaliburjs.github.io#57

## Changes:

- Moves existing drawing functionality to ECS
- Updates to ECS implementation to facilitate Excalibur integration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants