Unity3D Field

Welcome with your application.

I think your module is to small to get full project access, but you could apply for single project access, but I think you need to develop it a little bit more. Read more about how much code you need over here: http://groups.drupal.org/node/195848

Anyhow, you have some coding standards issues to take care about:
http://ventral.org/pareview/httpgitdrupalorgsandboxroycekimmons1424978git

Hello, first off used the online auto scanner. Found some issues. These are included in the attached file. Feel free to run the scanner yourself to resolve issues.

For Drupal coding conventions please refer to:
http://drupal.org/coding-standards#comment
http://drupal.org/node/1354

There is a readme file. A separate installation text file is a good measure.

In the unity3d_logo.tpl file there is much logic for a template file. It is good to put this in pre-process functions or template functions as needed. Also one should build images using the theme('image' call. The template should be simple HTML with embedded if ( something ) and so forth. Same is true for unity3d_field.tpl.

For the .module file the same is also true with lines:

// Show a preview of the current default image.
  $image = '<img alt="' . $alt_text . '" title="' . $alt_text . '"';
  $image .= ' src="' . file_create_url($image_path) . '">';

also

0 => '<img src="' . file_create_url($default_path) . '">',

You have a validate function present for the form, that is good.

Thank you.

Hi Royce!

You should merge your install.txt into your readme.txt. Also your project page could need some more details. At least it should contain a list of the dependencies the module has.

In unity3d_field_settings_form you are using check_plain to validate the translated description. I am not sure if that is needed. Anyway it shouldn't cost much performance if it is kept.

Your validation code might be a little less complex.

  if (count($dimensions) == 2) {
    if (is_numeric($dimensions[0]) && is_numeric($dimensions[1])) {
      $valid = TRUE;
    }
  }
  if (!$valid) {
    $error_msg = t('You must enter valid dimensions to continue.');
    form_set_error('dimensions', $error_msg);
  }

Might be written as

  if ((count($dimensions) != 2) || !is_numeric($dimensions[0]) !is_numeric($dimensions[1])) {
      $error_msg = t('You must enter valid dimensions to continue.');
      form_set_error('dimensions', $error_msg);
    }
  }

Same for the default image and for unity3d_field_validate_image.

I have never themed something, but this feels to complicated:

foreach ($items as $item) :
  ?>
<?php echo $item['image']; ?>
<?php endforeach; ?>

Isn't it possible to use this instead?

foreach ($items as $item) {
  echo $item['image'];
}

Tiny automatic review error found:

 230 | ERROR | Line indented incorrectly; expected 2 spaces, found 4

This seem ready to go. I think it uses the drupal api in a good way.

1. Your project page is very short, please include a more detailed description, usage and installation instructions (also see tips for a great project page).
2. t($description), This won't be translatable as only strings that are directly in the function call (like t('translate me')) will be detected by the translation server.
3. line 70: 'Preview' - you should put this through t()
4. some of your settings can be saved automatically, just put system_settings_submit() into your submit handler (that makes it easier to add other variables later)
5. Your setting variables but you don't remove them with variable_del() in a hook_uninstall() in .install
6. Please use hook_preprocess_HOOK instead of hook_preprocess() ( It is called for all invocations of theme(), what is not very good for performance)
7. Set the proper url depending upon the entity type. This is not the best way to go, as this will only work for nodes and users - use entity_uri() instead
8. (BTW you don't have to split up strings, just to have them smaller then 80 characters per line if it makes you code look ugly)
9. $error_msg = t('The file is not a valid image.'); form_error($element, check_plain($error_msg)); Don't use filtering on static strings in your module. You only have to filter all user input before printing it out. if your including variables within t() and you use @/% properly you don't have to do any further filtering.
10. Whenever possible, try to include no html within t(), that will make the live of translators easier
11. Some 0/1 looks like they'r booleans, if they are, please replace them with TRUE/FALSE

Leaving RTBC, I think this is ready after you fixed the points 1, 5. Highly recommended: 2, 6, 7, 9.

Great!

Thanks for your contribution and welcome to the community of project contributors on drupal.org!

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.