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

Tile Objects with Type attribute already assigned #1248

Closed
Seanba opened this issue Apr 11, 2016 · 10 comments
Closed

Tile Objects with Type attribute already assigned #1248

Seanba opened this issue Apr 11, 2016 · 10 comments
Assignees
Labels
missing feature It's not just a feature, it's a feature that really should be there! usability Generally about making something more intuitive or efficient.
Milestone

Comments

@Seanba
Copy link
Contributor

Seanba commented Apr 11, 2016

Loving the new Object Types stuff. For Tile Objects in particular I think it would be useful if we could have the Type attribute assigned to the Tile (maybe through the properties of the Tile?) so that we could drop Tile Objects down in our maps with the "Type" already filled out.

@todorone
Copy link

Million of upvotes for this feature, the latest Types Editor feature is amazing but without auto assigning of types to objects the process of manual assigning of type after object placing to map is very slow...

@vovnet
Copy link

vovnet commented May 4, 2016

Need this!

@yozka
Copy link

yozka commented May 4, 2016

add

@bjorn
Copy link
Member

bjorn commented May 4, 2016

I can totally see the usefulness of this. What I'm not sure about are the details of this change. I could imagine the following:

  • Add a Type built-in property to the Tile class.
  • When a tile is selected, display default properties based on this type just like what happens for objects.
  • When a tile object is selected, take its type from the tile if it is not set for the object itself. Then, display default properties based on the type, after overriding them with any properties set on the tile.

Does that make sense? Or should, in the case of a type being set on both the object and the tile, the default properties of these types be merged as well? In that case there are three places where default properties for the object could come from (the two types and the tile properties), and I wonder in which order they should override each other.

Opinions welcome.

@todorone
Copy link

todorone commented May 5, 2016

Just an opinion.
For me an idea with any type of merging is not very good. Although it's more flexible, it may result in lot of hard tracked bugs. I guess:

  • Add a Type built-in property to the Tile class.
  • When a tile is selected, display default properties based on this type just like what happens for objects. Maybe even to forbid edit custom fields when tile "type" is set - if you want to edit custom fields, "type" field must be empty.
  • When a tile object is selected, take its type from the tile if it's not set for the object itself. Then, display default properties based on the type allowing to add additional custom vars.

Let me explain why so strict. For example - i have game with 20 levels(maps) using object types set. While editing one of the map, i can accidently start to change custom variables of tile in tile editor window, but it means than i will need to change this vars in all maps. The good practice here is to change custom vars in Object Type Editor thus changing vars of particular object in all maps. So i think this restriction will lead to much better organisation of multi map solutions.
Sorry if it sounds unclear due to awful english..:)

@bjorn
Copy link
Member

bjorn commented May 5, 2016

@todorone Thanks for the feedback! I think you're right that we probably shouldn't make this too complex. The use-case I was seeing is the one where somebody doesn't use the "Type" field at all, but just adds custom properties to his tiles. When doing this, some people expect to see them and be able to override them when having a tile object selected (issue #436).

@bjorn bjorn added missing feature It's not just a feature, it's a feature that really should be there! Urgent usability Generally about making something more intuitive or efficient. labels Oct 25, 2016
@bjorn
Copy link
Member

bjorn commented Feb 6, 2017

Similar request of inheriting the name from the tile posted on the forum.

@bjorn bjorn added this to the Tiled 1.0 milestone Feb 13, 2017
@bjorn
Copy link
Member

bjorn commented Feb 21, 2017

I just realized this issue has a duplicate as #432.

@bjorn bjorn self-assigned this Feb 21, 2017
@bjorn
Copy link
Member

bjorn commented Feb 21, 2017

I think I'll have to go with the complex approach with multiple levels of overriding. Example use case:

  • You define your object types at a granularity of "NPC", "Enemy", "Item".
  • You have multiple tiles representing each of these object types.
  • You instantiate multiple instances of those tiles on various maps.

Here, it would really suck if you could only define stuff like "name", "health" or "weight" at the object instance level. You can't do it at the object type level either, so really it should be done at the tile level. I think this is common for most use-case of tile objects.

So I think the model that makes sense is to just add a level of overriding to the custom properties. For any selected tile object:

  • First of all, the properties based on its type (or when left empty, the type of its tile) are taken
  • Override with any properties specified on its tile
  • Override with any properties specified on the object itself

In the UI, I will use black when the property is set on the currently selected object and gray (disabled color) when it is inherited. In the future, it may be helpful to reserve gray for signaling different values for that property on "other selected objects" and use something else, like italics, for inherited values. Or to display values explicitly set on the currently selected object in bold, like Qt Designer does.

On the long term, it probably make sense to re-think the way we do objects and instantiation in Tiled, but in the current version I think the above behavior will work fine.

@bjorn bjorn closed this as completed in 5ab9bb9 Feb 27, 2017
@bjorn
Copy link
Member

bjorn commented Apr 18, 2017

This new behavior is now documented on the update page about Custom Properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
missing feature It's not just a feature, it's a feature that really should be there! usability Generally about making something more intuitive or efficient.
Projects
None yet
Development

No branches or pull requests

5 participants