Skip to content

Map object by name#50

Open
GryphonClaw wants to merge 3 commits into
wildfiler:masterfrom
GryphonClaw:map-object_by_name
Open

Map object by name#50
GryphonClaw wants to merge 3 commits into
wildfiler:masterfrom
GryphonClaw:map-object_by_name

Conversation

@GryphonClaw

Copy link
Copy Markdown
Contributor

As discussed. Added a method to the map class that allows you to look up an object by name, given a specific name for a map/object layer.

If the object layer named can't be found, it will raise an ObjectLayerNotFound error. giving the name of the object layer passed and which map (via the path) couldn't find it.

if that doesn't fail, the method then checks for an object in the layer with the passed name.

if the object can't be found, it will raise an ObjectNameNotFound error, giving the name of the object that couldn't be found on the passed layer name.

get current with main project
Adds the object_by_name method to the Tiled::Map class as discussed.
added the following exceptions, which are raised in the object_by_name of the Tiled::Map class
 ObjectLayerNotFound and ObjectNameNotFound
Comment thread lib/tiled/map.rb
#then returns that object
def object_by_name(layer_name, object_name)

named_layer = @object_groups.reject do | layer |

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It is better to use object_groups method instead of instance variable, because it can be uninitialized if map file doesn't have object layers.

object_group is a Tiled::Layers instance, so you can work with it like with map.layers:

named_layer = object_groups[layer_name]

As a bonus it will cache reference to this layer.

Comment thread lib/tiled/map.rb

raise ObjectLayerNotFound, "The object layer \"#{layer_name}\" was not found in map \"#{@path}\"" if named_layer.nil?

the_object = named_layer.objects.reject do | object |

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

named_layer will be a Tiled::ObjectLayer and has find_by_name method, so you can use it to find object by name:

the_object = named_layer.find_by_name(object_name)

It will always return first object or nil if object is not found.

Comment thread lib/tiled/tiled.rb
class MapNotFound < Error; end
class TilesetNotFound < Error; end

#added the following exceptions, which are raised in the object_by_name of the Tiled::Map class

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think class names are self-explanatory so no need for comment here.

Comment thread lib/tiled/map.rb
return height * tileheight
end

#gets the first object layer with the passed name of `layer_name`

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I would prefer to format comments in yard syntax, you can check the example in the lib/tiled/layers.rb file.

Comment thread lib/tiled/map.rb
#gets the first object layer with the passed name of `layer_name`
#then looks in that layer for the first object that has the name `object_name`
#then returns that object
def object_by_name(layer_name, object_name)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm not sure about passing a layer name there, because this action can be performed in this way:

map.layers[layer_name].find_by_name(object_name)

But it would be more valuable if the user will be able to find an object without passing layer_name:

map.object_by_name(object_name)

What do you think about this?

Comment thread lib/tiled/map.rb
layer.name != layer_name
end.first

raise ObjectLayerNotFound, "The object layer \"#{layer_name}\" was not found in map \"#{@path}\"" if named_layer.nil?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Right now all methods return nil if desired object/tile is not found, it is better to save this approach.

But I think it is good to have a method that raises errors in case an object is not found. I love the approach when the regular method is returning nil and the bang method raises errors:

def object_by_name(object_name)
  # return nil and not raise errors
end

def object_by_name!(object_name)
  raise SomeError if the_object.nil?
end

Do you think it makes sense?

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.

Yeah, I think that makes sense, staying consistent will keep people from getting confused. I will look into it.

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.

I think with the release of v0.4.0 this can be closed for now.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants