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

Processing recipe rework #3232

Merged
merged 1 commit into from
Jun 24, 2018
Merged

Processing recipe rework #3232

merged 1 commit into from
Jun 24, 2018

Conversation

fscan
Copy link
Member

@fscan fscan commented Nov 17, 2017

Work in progress!

Processing recipes based on json recipe parsing.

*) Removed recipes for external mods for now. If we really need it we can add it back easy enough.
*) Removed WebSerializer. This probably has to be reimplemented.
*) The old recipe parser still has some stuff in the API

private boolean process( final Path root, final Path file )
{
String relative = root.relativize( file ).toString();
if( !"json".equals( FilenameUtils.getExtension( file.toString() ) ) || relative.startsWith( "_" ) )

Choose a reason for hiding this comment

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

MINOR Missing curly brace. rule


private void register( JsonObject json )
{
if( json == null || json.isJsonNull() )

Choose a reason for hiding this comment

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

MINOR Missing curly brace. rule

public class AERecipeLoader
{
private static final String AERECIPE_BASE = "/aerecipes";
private static Gson GSON = new GsonBuilder().setPrettyPrinting().disableHtmlEscaping().create();

Choose a reason for hiding this comment

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

MINOR Rename this field "GSON" to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule

return CraftingHelper.findFiles( this.mod, "assets/" + AppEng.MOD_ID + AERECIPE_BASE, this::preprocess, this::process, true, true );
}

private boolean preprocess( final Path root )

Choose a reason for hiding this comment

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

MAJOR Remove this unused method parameter "root". rule

throw new JsonSyntaxException( "Json cannot be null" );

String type = this.ctx.appendModId( JsonUtils.getString( json, "type" ) );
if( type.isEmpty() )

Choose a reason for hiding this comment

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

MINOR Missing curly brace. rule

{
reader = Files.newBufferedReader( file );
JsonObject json = JsonUtils.fromJson( GSON, reader, JsonObject.class );
if( json.has( "conditions" ) && !CraftingHelper.processConditions( JsonUtils.getJsonArray( json, "conditions" ), this.ctx ) )

Choose a reason for hiding this comment

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

MINOR Missing curly brace. rule

throw new JsonSyntaxException( "Recipe type can not be an empty string" );

IAERecipeFactory factory = factories.get( new ResourceLocation( type ) );
if( factory == null )

Choose a reason for hiding this comment

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

MINOR Missing curly brace. rule

{
Preconditions.checkNotNull( event );
Preconditions.checkNotNull( recipeDirectory );
Preconditions.checkArgument( !recipeDirectory.isFile() );
Preconditions.checkNotNull( customRecipeConfig );

final Api api = Api.INSTANCE;
final IPartHelper partHelper = api.partHelper();

Choose a reason for hiding this comment

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

MAJOR Remove this useless assignment to local variable "partHelper". rule
MINOR Remove this unused "partHelper" local variable. rule

@Override
public void register( JsonObject json, JsonContext ctx )
{
// TODO only primary for now

Choose a reason for hiding this comment

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

INFO Complete the task associated to this TODO comment. rule

import net.minecraftforge.common.crafting.JsonContext;


public interface IAERecipeFactory

Choose a reason for hiding this comment

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

CRITICAL Annotate the "IAERecipeFactory" interface with the @FunctionalInterface annotation rule

@@ -74,696 +31,15 @@
*/
public class RecipeHandler implements IRecipeHandler

Choose a reason for hiding this comment

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

MINOR Remove this use of "IRecipeHandler"; it is deprecated. rule


return in;
}

@Override
public void parseRecipes( final IRecipeLoader loader, final String path )

Choose a reason for hiding this comment

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

MINOR Remove this use of "IRecipeLoader"; it is deprecated. rule

@fscan fscan added this to the rv6.alpha - ? milestone Nov 22, 2017
@fscan fscan changed the base branch from rv5-1.12 to rv6-1.12 January 15, 2018 20:09
@@ -183,11 +183,12 @@ public boolean isCable()

QUARTZ_FIBER( 140, "quartz_fiber", EnumSet.of( AEFeature.QUARTZ_FIBER ), EnumSet.noneOf( IntegrationType.class ), PartQuartzFiber.class ),

MONITOR( 160, "monitor", EnumSet.of( AEFeature.PANELS ), EnumSet.noneOf( IntegrationType.class ), PartPanel.class ),
MONITOR( 160, "monitor", EnumSet.of( AEFeature.PANELS ), EnumSet.noneOf( IntegrationType.class ), PartPanel.class, "itemIlluminatedPanel" ),

Choose a reason for hiding this comment

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

CRITICAL Define a constant instead of duplicating this literal "itemIlluminatedPanel" 3 times. rule

@fscan
Copy link
Member Author

fscan commented Feb 10, 2018

Rebased (and squashed to make it easier).
We still need to take a closer look at the exposed API. As this is now for rv6 only, changes are possible.

@orod-org
Copy link

SonarQube analysis reported 79 issues

  • BLOCKER 5 blocker
  • CRITICAL 11 critical
  • MAJOR 9 major
  • MINOR 52 minor
  • INFO 2 info

Watch the comments in this conversation to review them.

Top 10 extra issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. BLOCKER AERecipeLoader.java#: Add or update the header of this file. rule
  2. BLOCKER IAERecipeFactory.java#: Add or update the header of this file. rule
  3. BLOCKER GrinderHandler.java#: Add or update the header of this file. rule
  4. BLOCKER InscriberHandler.java#: Add or update the header of this file. rule
  5. BLOCKER SmeltingHandler.java#: Add or update the header of this file. rule
  6. CRITICAL ApiBlocks.java#L292: Define a constant instead of duplicating this literal "sky_stone_chest" 3 times. rule
  7. CRITICAL ApiBlocks.java#L351: Define a constant instead of duplicating this literal "quantum_ring" 3 times. rule
  8. CRITICAL ApiBlocks.java#L435: Define a constant instead of duplicating this literal "crafting_unit" 4 times. rule
  9. CRITICAL ApiBlocks.java#L447: Define a constant instead of duplicating this literal "crafting_storage" 4 times. rule
  10. CRITICAL ItemPart.java#L247: Define a constant instead of duplicating this literal "Unable to construct IBusPart from IBusItem : " 4 times. rule

@fscan fscan merged commit 841bc6e into rv6-1.12 Jun 24, 2018
@fscan fscan deleted the aerecipe-rework branch July 14, 2018 17:25
@rehv rehv mentioned this pull request Jun 1, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants