Skip to content

Modify startAvatars to reflect custom cell sprites#9705

Merged
balderdash merged 3 commits into
stagingfrom
dropdown-sprites
Jul 26, 2016
Merged

Modify startAvatars to reflect custom cell sprites#9705
balderdash merged 3 commits into
stagingfrom
dropdown-sprites

Conversation

@balderdash

@balderdash balderdash commented Jul 26, 2016

Copy link
Copy Markdown
Contributor

startAvatars is no longer reordered based on level.firstSpriteIndex, instead the sprite images are assigned using i + level.firstSpriteIndex instead of just i. This also fixes a bug where custom cell sprites were being affected by the offset when they should not have been.

The block initialization code now just receives a separately constructed list of sprites that reflects the exact set of sprites available in the level, making the dropdowns accurate.

Comment thread apps/src/studio/studio.js Outdated
});
if (cell.sprite !== undefined) {
presentAvatars[Studio.spriteCount] =
skin.avatarList[cell.sprite + level.firstSpriteIndex];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't we just want this to be cell.sprite? I thought the assigned sprites did not respect the offset

@balderdash balderdash Jul 26, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They did respect the offset, but I guess that was a bug (particularly since they weren't shown to follow it in the level editor).

Fixed that, and checked that there are no published levels that use both an offset and assigned sprites, so I'm not messing up any existing levels by changing the displayed sprite.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, dang, right you are! That was definitely an oversight on my part, thanks for catching it.

@Hamms

Hamms commented Jul 26, 2016

Copy link
Copy Markdown
Contributor

one question, otherwise LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants