ronantakizawa/codereview-bench huggingface dataset:
Repository: 4ian/GDevelop
File: GDJS/tests/tests/Extensions/testspriteruntimeobject.js (JavaScript)

Code before:
  }

  setCustomCenter(customCenterX, customCenterY) {
    this._customCenterX = customCenterX;
    this._customCenterY = customCenterY;
    this.invalidateHitboxes();
  }

  getRendererObject() {
    return null;
  }

  getWidth() {
    return this._customWidth;
  }

  getHeight() {
    return this._customHeight;
  }

  setWidth(width) {
    this._customWidth = width;
  }

  setHeight(height) {
    return this._customHeight = height;
  }

  getCenterX() {
    if (this._customCenterX === null) return super.getCenterX();
    return this._customCenterX;
  }

  getCenterY() {
    if (this._customCenterY === null) return super.getCenterY();
    return this._customCenterY;
  }
};

gdjs.registerObject('TestSpriteObject::TestSpriteObject', gdjs.TestSpriteRuntimeObject);


Review comment (suggestion):
```suggestion
   this._customHeight = height;
```

Code after:
  }

  setCustomCenter(customCenterX, customCenterY) {
    this._customCenterX = customCenterX;
    this._customCenterY = customCenterY;
    this.invalidateHitboxes();
  }

  getRendererObject() {
    return null;
  }

  getWidth() {
    return this._customWidth;
  }

  getHeight() {
    return this._customHeight;
  }

  setWidth(width) {
    this._customWidth = width;
  }

  setHeight(height) {
    this._customHeight = height;
  }

  getCenterX() {
    if (this._customCenterX === null) return super.getCenterX();
    return this._customCenterX;
  }

  getCenterY() {
    if (this._customCenterY === null) return super.getCenterY();
    return this._customCenterY;
  }
};

gdjs.registerObject('TestSpriteObject::TestSpriteObject', gdjs.TestSpriteRuntimeObject);
Repository: 4ian/GDevelop
File: newIDE/app/src/Utils/GetObjectByName.js (JavaScript)

Code before:
// @flow

export default function getObjectByName(
  globalObjectsContainer: gdObjectsContainer | null,
  objectsContainer?: ?gdObjectsContainer,
  objectName: string
): ?gdObject {
  if (objectsContainer && objectsContainer.hasObjectNamed(objectName))
    return objectsContainer.getObject(objectName);
  else if (
    globalObjectsContainer &&
    globalObjectsContainer.hasObjectNamed(objectName)
  )
    return globalObjectsContainer.getObject(objectName);

  return null;
}

export const hasObjectWithName = (
  globalObjectsContainer: gdObjectsContainer | null,
  objectsContainer?: ?gdObjectsContainer,
  objectName: string
): boolean => {
  if (objectsContainer && objectsContainer.hasObjectNamed(objectName))
    return true;
  else if (
    globalObjectsContainer &&
    globalObjectsContainer.hasObjectNamed(objectName)
  )
    return true;
  return false;
};


Review comment (suggestion):
`ObjectsContainersList` should be used instead. You can pass a `ProjectScopedContainersAccessor` to your component.

Code after:
// @flow

export default function getObjectByName(
  globalObjectsContainer: gdObjectsContainer | null,
  objectsContainer?: ?gdObjectsContainer,
  objectName: string
): ?gdObject {
  if (objectsContainer && objectsContainer.hasObjectNamed(objectName))
    return objectsContainer.getObject(objectName);
  else if (
    globalObjectsContainer &&
    globalObjectsContainer.hasObjectNamed(objectName)
  )
    return globalObjectsContainer.getObject(objectName);

  return null;
}
Repository: 4ian/GDevelop
File: Core/GDCore/Project/ProjectScopedContainers.h (C/C++)

Code before:
  /**
   * @brief Return the scene variables of the current scene or the current
   * extension. It allows legacy "scenevar" parameters to accept extension
   * variables.
   */
  const gd::VariablesContainer &GetLegacySceneVariables() const {
    return legacySceneVariables;
  };

  const gd::PropertiesContainersList &GetPropertiesContainersList() const {
    return propertiesContainersList;
  };

  const std::vector<const ParameterMetadataContainer *> &GetParametersVectorsList() const {
    return parametersVectorsList;
  };

  /** Do not use - should be private but accessible to let Emscripten create a
   * temporary. */
  ProjectScopedContainers(){};

 private:
  gd::ObjectsContainersList objectsContainersList;
  gd::VariablesContainersList variablesContainersList;
  gd::VariablesContainer legacyGlobalVariables;
  gd::VariablesContainer legacySceneVariables;
  gd::PropertiesContainersList propertiesContainersList;
  std::vector<const ParameterMetadataContainer *> parametersVectorsList;
};

}  // namespace gd

Review comment (suggestion):
These are not references/pointers but values, meaning that ProjectScopedContainers moved from "I'm just a set of lists pointing to things in your project" to "I actually hold stuff, and you will have a bad time if you destroyed me and kept references to things".

Code after:
  /**
   * @brief Return the scene variables of the current scene or the current
   * extension. It allows legacy "scenevar" parameters to accept extension
   * variables.
   */
  const gd::VariablesContainer *GetLegacySceneVariables() const {
    return legacySceneVariables;
  };

  const gd::PropertiesContainersList &GetPropertiesContainersList() const {
    return propertiesContainersList;
  };

  const std::vector<const ParameterMetadataContainer *> &GetParametersVectorsList() const {
    return parametersVectorsList;
  };

  /** Do not use - should be private but accessible to let Emscripten create a
   * temporary. */
  ProjectScopedContainers(){};

 private:
  gd::ObjectsContainersList objectsContainersList;
  gd::VariablesContainersList variablesContainersList;
  const gd::VariablesContainer *legacyGlobalVariables;
  const gd::VariablesContainer *legacySceneVariables;
  gd::PropertiesContainersList propertiesContainersList;
  std::vector<const ParameterMetadataContainer *> parametersVectorsList;
};

}  // namespace gd
Repository: 4ian/GDevelop
File: newIDE/app/src/EventsFunctionsList/EventsBasedObjectTreeViewItemContent.js (JavaScript)

Code before:
        type: 'separator',
      },
      {
        label: i18n._(t`Copy`),
        click: () => this.copy(),
        accelerator: 'CmdOrCtrl+C',
      },
      {
        label: i18n._(t`Cut`),
        click: () => this.cut(),
        accelerator: 'CmdOrCtrl+X',
      },
      {
        label: i18n._(t`Paste`),
        enabled: Clipboard.has(EVENTS_BASED_OBJECT_CLIPBOARD_KIND),
        click: () => this.paste(),
        accelerator: 'CmdOrCtrl+V',
      },
    ].filter(Boolean);
  }

  renderRightComponent(i18n: I18nType): ?React.Node {
    return this.eventsBasedObject.isPrivate() ? (
      <Tooltip
        title={
          <Trans>This object won't be visible in the events editor.</Trans>
        }
      >
        <VisibilityOff
          fontSize="small"
          style={{
            ...styles.tooltip,
            color: this.props.gdevelopTheme.text.color.disabled,
          }}
        />
      </Tooltip>
    ) : null;
  }

  delete(): void {
    this._deleteEventsBasedObject({
      askForConfirmation: true,
    });
  }

  async _deleteEventsBasedObject({
    askForConfirmation,
  }: {|
    askForConfirmation: boolean,
  |}): Promise<void> {
    const { eventsBasedObjectsList } = this.props;

Review comment (suggestion):
```suggestion
          <Trans>This object won't be visible in the scene editor.</Trans>
```

Code after:
        type: 'separator',
      },
      {
        label: i18n._(t`Copy`),
        click: () => this.copy(),
        accelerator: 'CmdOrCtrl+C',
      },
      {
        label: i18n._(t`Cut`),
        click: () => this.cut(),
        accelerator: 'CmdOrCtrl+X',
      },
      {
        label: i18n._(t`Paste`),
        enabled: Clipboard.has(EVENTS_BASED_OBJECT_CLIPBOARD_KIND),
        click: () => this.paste(),
        accelerator: 'CmdOrCtrl+V',
      },
    ].filter(Boolean);
  }

  renderRightComponent(i18n: I18nType): ?React.Node {
    return this.eventsBasedObject.isPrivate() ? (
      <Tooltip
        title={
          <Trans>
            This object won't be visible in the scene and events editors.
          </Trans>
        }
      >
        <VisibilityOff
          fontSize="small"
          style={{
            ...styles.tooltip,
            color: this.props.gdevelopTheme.text.color.disabled,
          }}
        />
      </Tooltip>
    ) : null;
  }

  delete(): void {
    this._deleteEventsBasedObject({
      askForConfirmation: true,
    });
  }

  async _deleteEventsBasedObject({
    askForConfirmation,
  }: {|
    askForConfirmation: boolean,
Repository: 4ian/GDevelop
File: Extensions/TextInput/JsExtension.js (JavaScript)

Code before:
        .setGroup(_('Border appearance'));

      objectProperties
        .getOrCreate('padding')
        .setValue((objectContent.padding || 0).toString())
        .setType('number')
        .setLabel(_('Padding'))
        .setGroup(_('Border appearance'));

      objectProperties
        .getOrCreate('maxLength')
        .setValue(objectContent.maxLength || 20)
        .setType('number')
        .setLabel(_('Max length'))
        .setGroup(_('Border appearance'));

      objectProperties
        .getOrCreate('textAlign')
        .setValue(objectContent.textAlign || 'left')
        .setType('choice')
        .addExtraInfo('left')
        .addExtraInfo('right')
        .addExtraInfo('center')
        .setLabel(_('text Alignement'))
        .setGroup(_('Border appearance'));

      return objectProperties;
    };
    textInputObject.content = {
      initialValue: '',
      placeholder: 'Touch to start typing',
      fontResourceName: '',
      fontSize: 20,
      inputType: 'text',
      textColor: '0;0;0',
      fillColor: '255;255;255',
      fillOpacity: 255,
      borderColor: '0;0;0',
      borderOpacity: 255,
      borderWidth: 1,
      readOnly: false,
      disabled: false,
      padding: 0,
      textAlign: 'left',
      maxLength: '20',
    };

    textInputObject.updateInitialInstanceProperty = function (
      instance,
      propertyName,
      newValue

Review comment (suggestion):
I think that the order is important here. I see that the text uses the order `left, center right`, so we might as well use the same here

Code after:
        .setGroup(_('Border appearance'));

      objectProperties
        .getOrCreate('padding')
        .setValue((objectContent.padding || 0).toString())
        .setType('number')
        .setLabel(_('Padding'))
        .setGroup(_('Font'));
      objectProperties
        .getOrCreate('maxLength')
        .setValue((objectContent.maxLength || 0).toString())
        .setType('number')
        .setLabel(_('Max length'))
        .setDescription(
          _(
            'The maximum length of the input value (this property will be ignored if the input type is a number).'
          )
        )
        .setAdvanced(true);

      objectProperties
        .getOrCreate('textAlign')
        .setValue(objectContent.textAlign || 'left')
        .setType('choice')
        .addExtraInfo('left')
        .addExtraInfo('center')
        .addExtraInfo('right')
        .setLabel(_('Text alignment'))
        .setGroup(_('Font'));

      return objectProperties;
    };
    textInputObject.content = {
      initialValue: '',
      placeholder: 'Touch to start typing',
      fontResourceName: '',
      fontSize: 20,
      inputType: 'text',
      textColor: '0;0;0',
      fillColor: '255;255;255',
      fillOpacity: 255,
      borderColor: '0;0;0',
      borderOpacity: 255,
      borderWidth: 1,
      readOnly: false,
      disabled: false,
      padding: 0,
      textAlign: 'left',
      maxLength: 0,
    };
Repository: 4ian/GDevelop
File: Extensions/TextInput/textinputruntimeobject.ts (TypeScript)

Code before:
    }

    getText() {
      return this._string;
    }

    setText(newString: string) {
      if (newString === this._string) return;

      this._string = newString;
      this._renderer.updateString();
    }

    /**
     * Called by the renderer when the value of the input shown on the screen
     * was changed (because the user typed something).
     * This does not propagate back the value to the renderer, which would
     * result in the cursor being sent back to the end of the text.
     *
     * Do not use this if you are not inside the renderer - use `setString` instead.
     */
    onRendererInputValueChanged(inputValue: string) {
      this._string = inputValue;
    }
    
    onRendererFormSubmitted(inputValue: boolean) {
      this._isSubmitted = inputValue;
    }

    getFontResourceName() {
      return this._fontResourceName;
    }

    setFontResourceName(resourceName: string) {
      if (this._fontResourceName === resourceName) return;

      this._fontResourceName = resourceName;
      this._renderer.updateFont();
    }

    getFontSize() {
      return this._fontSize;
    }

    setFontSize(newSize: number) {
      this._fontSize = newSize;
    }

    /**
     * Get the placeholder shown when no text is entered
     */

Review comment (suggestion):
```suggestion
    onRendererFormSubmitted() {
```

Code after:
     * @deprecated use `getText` instead
     */
    getString() {
      return this.getText();
    }

    /**
     * Replace the text inside the text input.
     * @deprecated use `setText` instead
     */
    setString(text: string) {
      this.setText(text);
    }

    getText() {
      return this._string;
    }

    setText(newString: string) {
      if (newString === this._string) return;

      this._string = newString;
      this._renderer.updateString();
    }

    /**
     * Called by the renderer when the value of the input shown on the screen
     * was changed (because the user typed something).
     * This does not propagate back the value to the renderer, which would
     * result in the cursor being sent back to the end of the text.
     *
     * Do not use this if you are not inside the renderer - use `setString` instead.
     */
    onRendererInputValueChanged(inputValue: string) {
      this._string = inputValue;
    }

    onRendererFormSubmitted() {
      this._isSubmitted = true;
    }

    getFontResourceName() {
      return this._fontResourceName;
    }

    setFontResourceName(resourceName: string) {
      if (this._fontResourceName === resourceName) return;

      this._fontResourceName = resourceName;
      this._renderer.updateFont();
    }
Repository: 4ian/GDevelop
File: Extensions/TextInput/textinputruntimeobject-pixi-renderer.ts (TypeScript)

Code before:

      this._input.style.borderColor = formatRgbAndOpacityToCssRgba(
        this._object._getRawBorderColor(),
        this._object.getBorderOpacity()
      );
    }
    updateBorderWidth() {
      if (!this._input) return;

      this._input.style.borderWidth = this._object.getBorderWidth() + 'px';
    }
    updateDisabled() {
      if (!this._form) return;

      this._form.disabled = this._object.isDisabled();
    }
    updateReadOnly() {
      if (!this._form) return;

      this._form.readOnly = this._object.isReadOnly();
    }

    updateMaxLength() {
      if (!this._input) return;
      if (this._object.getMaxLength() <= 0) {
        this._input?.removeAttribute('maxLength');
        return;
      }
      this._input.maxLength = this._object.getMaxLength();
    }

    updatePadding() {
      if (!this._input) return;

      this._input.style.padding = this._object.getPadding() + 'px';
    }

    updateTextAlign() {
      if (!this._input) return;

      const newTextAlign = this._object.getTextAlign() || 'left';
      this._input.style.textAlign = newTextAlign;
    }

    isFocused() {
      return this._input === document.activeElement;
    }

    focus() {
      if (!this._input) return;


Review comment (suggestion):
to avoid having the `?`, you could do this:
```js
const input = this._input;
if (!input) return;
// From then on, you can use `input` without TS complaining
```

Code after:
      );
    }

    updateBorderColorAndOpacity() {
      if (!this._input) return;

      this._input.style.borderColor = formatRgbAndOpacityToCssRgba(
        this._object._getRawBorderColor(),
        this._object.getBorderOpacity()
      );
    }
    updateBorderWidth() {
      if (!this._input) return;

      this._input.style.borderWidth = this._object.getBorderWidth() + 'px';
    }
    updateDisabled() {
      if (!this._form) return;

      this._form.disabled = this._object.isDisabled();
    }
    updateReadOnly() {
      if (!this._form) return;

      this._form.readOnly = this._object.isReadOnly();
    }

    updateMaxLength() {
      const input = this._input;
      if (!input) return;
      if (this._object.getMaxLength() <= 0) {
        input.removeAttribute('maxLength');
        return;
      }
      input.maxLength = this._object.getMaxLength();
    }

    updatePadding() {
      if (!this._input) return;

      this._input.style.padding = this._object.getPadding() + 'px';
    }

    updateTextAlign() {
      if (!this._input) return;

      const newTextAlign = this._object.getTextAlign();
      this._input.style.textAlign = newTextAlign;
    }

    isFocused() {
Repository: 4ian/GDevelop
File: Extensions/TextInput/textinputruntimeobject-pixi-renderer.ts (TypeScript)

Code before:

      // Display after the object is positioned.
      this._form.style.display = 'initial';
    }

    updateString() {
      if (!this._input) return;
      this._input.value = this._object.getString();
    }

    updatePlaceholder() {
      if (!this._input) return;
      this._input.placeholder = this._object.getPlaceholder();
    }

    updateFont() {
      if (!this._input) return;
      this._input.style.fontFamily = this._instanceContainer
        .getGame()
        .getFontManager()
        .getFontFamily(this._object.getFontResourceName());
    }

    updateOpacity() {
      if (!this._form) return;
      this._form.style.opacity = '' + this._object.getOpacity() / 255;
    }

    updateInputType() {
      if (!this._input) return;

      const isTextArea = this._input instanceof HTMLTextAreaElement;
      const shouldBeTextArea = this._object.getInputType() === 'text area';
      if (isTextArea !== shouldBeTextArea) {
        this._destroyElement();
        this._createElement();
      }

      const newType =
        userFriendlyToHtmlInputTypes[this._object.getInputType()] || 'text';
      this._input.setAttribute('type', newType);
    }

    updateTextColor() {
      if (!this._input) return;

      this._input.style.color = formatRgbAndOpacityToCssRgba(
        this._object._getRawTextColor(),
        255
      );
    }

Review comment (suggestion):
I think you could use `(this._object.getOpacity() / 255).toFixed(3)` instead. The result of the division could be `0.59200000000005`, and I'm not sure if CSS has some limitations about that.

Code after:

      // Display after the object is positioned.
      this._form.style.display = 'initial';
    }

    updateString() {
      if (!this._input) return;
      this._input.value = this._object.getString();
    }

    updatePlaceholder() {
      if (!this._input) return;
      this._input.placeholder = this._object.getPlaceholder();
    }

    updateFont() {
      if (!this._input) return;
      this._input.style.fontFamily = this._instanceContainer
        .getGame()
        .getFontManager()
        .getFontFamily(this._object.getFontResourceName());
    }

    updateOpacity() {
      if (!this._form) return;
      this._form.style.opacity = (this._object.getOpacity() / 255).toFixed(3);
    }

    updateInputType() {
      if (!this._input) return;

      const isTextArea = this._input instanceof HTMLTextAreaElement;
      const shouldBeTextArea = this._object.getInputType() === 'text area';
      if (isTextArea !== shouldBeTextArea) {
        this._destroyElement();
        this._createElement();
      }

      const newType =
        userFriendlyToHtmlInputTypes[this._object.getInputType()] || 'text';
      this._input.setAttribute('type', newType);
    }

    updateTextColor() {
      if (!this._input) return;

      this._input.style.color = formatRgbAndOpacityToCssRgba(
        this._object._getRawTextColor(),
        255
      );
    }
Repository: 4ian/GDevelop
File: Extensions/TextInput/textinputruntimeobject-pixi-renderer.ts (TypeScript)

Code before:
    updateReadOnly() {
      if (!this._form) return;

      this._form.readOnly = this._object.isReadOnly();
    }

    updateMaxLength() {
      const input = this._input;
      if (!input) return;
      if (this._object.getMaxLength() <= 0) {
        input.removeAttribute('maxLength');
        return;
      }
      input.maxLength = this._object.getMaxLength();
    }

    updatePadding() {
      if (!this._input) return;

      this._input.style.padding = this._object.getPadding() + 'px';
    }

    updateTextAlign() {
      if (!this._input) return;

      const newTextAlign = this._object.getTextAlign() || 'left';
      this._input.style.textAlign = newTextAlign;
    }

    isFocused() {
      return this._input === document.activeElement;
    }

    focus() {
      if (!this._input) return;

      this._input.focus();
    }
  }
  export const TextInputRuntimeObjectRenderer = TextInputRuntimeObjectPixiRenderer;
  export type TextInputRuntimeObjectRenderer = TextInputRuntimeObjectPixiRenderer;
}


Review comment (suggestion):
Is the `|| 'left'` necessary? I think we can be confident that the object only stores correct values

Code after:
    updateReadOnly() {
      if (!this._form) return;

      this._form.readOnly = this._object.isReadOnly();
    }

    updateMaxLength() {
      const input = this._input;
      if (!input) return;
      if (this._object.getMaxLength() <= 0) {
        input.removeAttribute('maxLength');
        return;
      }
      input.maxLength = this._object.getMaxLength();
    }

    updatePadding() {
      if (!this._input) return;

      this._input.style.padding = this._object.getPadding() + 'px';
    }

    updateTextAlign() {
      if (!this._input) return;

      const newTextAlign = this._object.getTextAlign();
      this._input.style.textAlign = newTextAlign;
    }

    isFocused() {
      return this._input === document.activeElement;
    }

    focus() {
      if (!this._input) return;

      this._input.focus();
    }
  }
  export const TextInputRuntimeObjectRenderer = TextInputRuntimeObjectPixiRenderer;
  export type TextInputRuntimeObjectRenderer = TextInputRuntimeObjectPixiRenderer;
}