morse-simulator / morse

The Modular OpenRobots Simulation Engine
http://morse-simulator.github.io/
Other
351 stars 155 forks source link

Corrected/improved the English + queries & comments #736

Closed mark-summerfield closed 7 years ago

mark-summerfield commented 7 years ago

On line 19 "It is however not needed in general." it isn't clear what "It" refers to (Blender file or specific logic). Maybe replace with "However, in general, a Blender file isn't needed." or "However, in general, no specific logic in the Blender file is needed."

Lines 141-147 I duplicated text rather than preserved the variety that was originally there. For literature you want the variety but for documentation you want uniformity for things that are uniform. (You rightly do this later on lines 338-342.)

Line 174 where does mathutils come from?

Line 253 - why isn't it: image = self.get_raw_image() # i.e., ending with ()?

I really dislike the add_data()/add_property()/add_level()/local_data API which I find very un-Pythonic.

It seems like you want people to save the new components they create inside MORSE's own directory tree. So what happens if they create a component that has the same name as a new built-in component and they update MORSE?

I don't recognize the word "revolute" on line 325. Should it be "revolving" or "bendable" or "movable"?

On line 382 you have Visible_arc but all the other params are lowercase, so is this correct?

You might like to consider using the flake8 tool to keep the code all uniform.

PierrickKoch commented 7 years ago

Hello and thank you for all the reviews!

about Visible_arc the properties system should be refactored, see #467

severin-lemaignan commented 7 years ago

Closed by c0b5854 Changes merged; other comments tracked in #744